[PATCH] drm/fbdev-dma: fix getting smem_start

[PATCH] drm/fbdev-dma: fix getting smem_start

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

If 'info->screen_buffer' locates in vmalloc address space, virt_to_page
will not be able to get correct results. With CONFIG_DEBUG_VM and
CONFIG_DEBUG_VIRTUAL enabled on ARM64, there is dump 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

So add a check 'is_vmalloc_addr'.

Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for GEM DMA helpers")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/gpu/drm/drm_fbdev_dma.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c
index 6c9427bb4053..9e2eddb6eb5c 100644
--- a/drivers/gpu/drm/drm_fbdev_dma.c
+++ b/drivers/gpu/drm/drm_fbdev_dma.c
@@ -130,7 +130,12 @@ 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 (is_vmalloc_addr(info->screen_buffer))
+		info->fix.smem_start = page_to_phys(vmalloc_to_page(info->screen_buffer));
+	else
+		info->fix.smem_start = page_to_phys(virt_to_page(info->screen_buffer));
+
 	info->fix.smem_len = info->screen_size;
 
 	return 0;
-- 
2.37.1

Re: [PATCH] drm/fbdev-dma: fix getting smem_start

[Thomas Zimmermann]

Hi

Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
> From: Peng Fan <peng.fan@nxp.com>
>
> If 'info->screen_buffer' locates in vmalloc address space, virt_to_page
> will not be able to get correct results. With CONFIG_DEBUG_VM and
> CONFIG_DEBUG_VIRTUAL enabled on ARM64, there is dump below:

Which graphics driver triggers this bug?

> [    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
>
> So add a check 'is_vmalloc_addr'.
>
> Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for GEM DMA helpers")
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Best regards
Thomas

> ---
>   drivers/gpu/drm/drm_fbdev_dma.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c
> index 6c9427bb4053..9e2eddb6eb5c 100644
> --- a/drivers/gpu/drm/drm_fbdev_dma.c
> +++ b/drivers/gpu/drm/drm_fbdev_dma.c
> @@ -130,7 +130,12 @@ 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 (is_vmalloc_addr(info->screen_buffer))
> +		info->fix.smem_start = page_to_phys(vmalloc_to_page(info->screen_buffer));
> +	else
> +		info->fix.smem_start = page_to_phys(virt_to_page(info->screen_buffer));
> +
>   	info->fix.smem_len = info->screen_size;
>   
>   	return 0;

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

RE: [PATCH] drm/fbdev-dma: fix getting smem_start

[Peng Fan]

> Subject: Re: [PATCH] drm/fbdev-dma: fix getting smem_start
> 
> Hi
> 
> Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > If 'info->screen_buffer' locates in vmalloc address space,
> > virt_to_page will not be able to get correct results. With
> > CONFIG_DEBUG_VM and CONFIG_DEBUG_VIRTUAL enabled on ARM64,
> there is dump below:
> 
> Which graphics driver triggers this bug?

It is NXP i.MX95 DPU driver which is still in NXP downstream repo.

Thanks,
Peng.

Re: [PATCH] drm/fbdev-dma: fix getting smem_start

[Thomas Zimmermann]

Hi

Am 11.06.24 um 03:00 schrieb Peng Fan:
>> Subject: Re: [PATCH] drm/fbdev-dma: fix getting smem_start
>>
>> Hi
>>
>> Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
>>> From: Peng Fan <peng.fan@nxp.com>
>>>
>>> If 'info->screen_buffer' locates in vmalloc address space,
>>> virt_to_page will not be able to get correct results. With
>>> CONFIG_DEBUG_VM and CONFIG_DEBUG_VIRTUAL enabled on ARM64,
>> there is dump below:
>>
>> Which graphics driver triggers this bug?
> It is NXP i.MX95 DPU driver which is still in NXP downstream repo.

Which DRM memory manager does that driver use?

I'm asking because if the driver allocates BOs via vmalloc(), should it 
really use fbdev-dma?

Best regards
Thomas

>
> Thanks,
> Peng.

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

RE: [PATCH] drm/fbdev-dma: fix getting smem_start

[Peng Fan]

> Subject: Re: [PATCH] drm/fbdev-dma: fix getting smem_start
> 
> Hi
> 
> Am 11.06.24 um 03:00 schrieb Peng Fan:
> >> Subject: Re: [PATCH] drm/fbdev-dma: fix getting smem_start
> >>
> >> Hi
> >>
> >> Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
> >>> From: Peng Fan <peng.fan@nxp.com>
> >>>
> >>> If 'info->screen_buffer' locates in vmalloc address space,
> >>> virt_to_page will not be able to get correct results. With
> >>> CONFIG_DEBUG_VM and CONFIG_DEBUG_VIRTUAL enabled on ARM64,
> >> there is dump below:
> >>
> >> Which graphics driver triggers this bug?
> > It is NXP i.MX95 DPU driver which is still in NXP downstream repo.
> 
> Which DRM memory manager does that driver use?

DRM_GEM_DMA_DRIVER_OPS

> 
> I'm asking because if the driver allocates BOs via vmalloc(), should it really
> use fbdev-dma?

We use it for fbdev emulation. If drm_kms_helper.fbdev_emulation is 0,
fbdev emulation could be disabled.

Thanks
Peng
> 
> Best regards
> Thomas
> 
> >
> > Thanks,
> > Peng.
> 
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB
> 36809 (AG Nuernberg)

Re: [PATCH] drm/fbdev-dma: fix getting smem_start

[Thomas Zimmermann]

Hi

Am 11.06.24 um 10:23 schrieb Peng Fan:
>> Subject: Re: [PATCH] drm/fbdev-dma: fix getting smem_start
>>
>> Hi
>>
>> Am 11.06.24 um 03:00 schrieb Peng Fan:
>>>> Subject: Re: [PATCH] drm/fbdev-dma: fix getting smem_start
>>>>
>>>> Hi
>>>>
>>>> Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
>>>>> From: Peng Fan <peng.fan@nxp.com>
>>>>>
>>>>> If 'info->screen_buffer' locates in vmalloc address space,
>>>>> virt_to_page will not be able to get correct results. With
>>>>> CONFIG_DEBUG_VM and CONFIG_DEBUG_VIRTUAL enabled on ARM64,
>>>> there is dump below:
>>>>
>>>> Which graphics driver triggers this bug?
>>> It is NXP i.MX95 DPU driver which is still in NXP downstream repo.
>> Which DRM memory manager does that driver use?
> DRM_GEM_DMA_DRIVER_OPS

So fbdev-dma would be correct. But with the gem-dma allocator, wouldn't 
you allocate from one of these dma_alloc_() calls at [1] ? How does the 
drivers end up with vmalloc'd fbdev memory? Specifically in the light of 
the docs at [2].

[1] 
https://elixir.bootlin.com/linux/v6.9/source/drivers/gpu/drm/drm_gem_dma_helper.c#L146
[2] 
https://elixir.bootlin.com/linux/v6.9/source/Documentation/core-api/dma-api-howto.rst#L124

Best regards
Thomas

>
>> I'm asking because if the driver allocates BOs via vmalloc(), should it really
>> use fbdev-dma?
> We use it for fbdev emulation. If drm_kms_helper.fbdev_emulation is 0,
> fbdev emulation could be disabled.
>
> Thanks
> Peng
>> Best regards
>> Thomas
>>
>>> Thanks,
>>> Peng.
>> --
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB
>> 36809 (AG Nuernberg)

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH] drm/fbdev-dma: fix getting smem_start

[Daniel Vetter]

On Tue, Jun 11, 2024 at 11:29:58AM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 11.06.24 um 10:23 schrieb Peng Fan:
> > > Subject: Re: [PATCH] drm/fbdev-dma: fix getting smem_start
> > > 
> > > Hi
> > > 
> > > Am 11.06.24 um 03:00 schrieb Peng Fan:
> > > > > Subject: Re: [PATCH] drm/fbdev-dma: fix getting smem_start
> > > > > 
> > > > > Hi
> > > > > 
> > > > > Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
> > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > 
> > > > > > If 'info->screen_buffer' locates in vmalloc address space,
> > > > > > virt_to_page will not be able to get correct results. With
> > > > > > CONFIG_DEBUG_VM and CONFIG_DEBUG_VIRTUAL enabled on ARM64,
> > > > > there is dump below:
> > > > > 
> > > > > Which graphics driver triggers this bug?
> > > > It is NXP i.MX95 DPU driver which is still in NXP downstream repo.
> > > Which DRM memory manager does that driver use?
> > DRM_GEM_DMA_DRIVER_OPS
> 
> So fbdev-dma would be correct. But with the gem-dma allocator, wouldn't you
> allocate from one of these dma_alloc_() calls at [1] ? How does the drivers
> end up with vmalloc'd fbdev memory? Specifically in the light of the docs at
> [2].

I think when you have an iommu dma_alloc just allocates pages, and uses
the iommu to make it all contiguous for the device, and vmalloc for the
kernel. So it's not allocated with vmalloc(), just ends up getting mapped
into the vmalloc range.
-Sima


> 
> [1] https://elixir.bootlin.com/linux/v6.9/source/drivers/gpu/drm/drm_gem_dma_helper.c#L146
> [2] https://elixir.bootlin.com/linux/v6.9/source/Documentation/core-api/dma-api-howto.rst#L124
> 
> Best regards
> Thomas
> 
> > 
> > > I'm asking because if the driver allocates BOs via vmalloc(), should it really
> > > use fbdev-dma?
> > We use it for fbdev emulation. If drm_kms_helper.fbdev_emulation is 0,
> > fbdev emulation could be disabled.
> > 
> > Thanks
> > Peng
> > > Best regards
> > > Thomas
> > > 
> > > > Thanks,
> > > > Peng.
> > > --
> > > --
> > > Thomas Zimmermann
> > > Graphics Driver Developer
> > > SUSE Software Solutions Germany GmbH
> > > Frankenstrasse 146, 90461 Nuernberg, Germany
> > > GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB
> > > 36809 (AG Nuernberg)
> 
> -- 
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
> 

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

Re: [PATCH] drm/fbdev-dma: fix getting smem_start

[Thomas Zimmermann]

Hi

Am 12.06.24 um 10:45 schrieb Daniel Vetter:
> On Tue, Jun 11, 2024 at 11:29:58AM +0200, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 11.06.24 um 10:23 schrieb Peng Fan:
>>>> Subject: Re: [PATCH] drm/fbdev-dma: fix getting smem_start
>>>>
>>>> Hi
>>>>
>>>> Am 11.06.24 um 03:00 schrieb Peng Fan:
>>>>>> Subject: Re: [PATCH] drm/fbdev-dma: fix getting smem_start
>>>>>>
>>>>>> Hi
>>>>>>
>>>>>> Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
>>>>>>> From: Peng Fan <peng.fan@nxp.com>
>>>>>>>
>>>>>>> If 'info->screen_buffer' locates in vmalloc address space,
>>>>>>> virt_to_page will not be able to get correct results. With
>>>>>>> CONFIG_DEBUG_VM and CONFIG_DEBUG_VIRTUAL enabled on ARM64,
>>>>>> there is dump below:
>>>>>>
>>>>>> Which graphics driver triggers this bug?
>>>>> It is NXP i.MX95 DPU driver which is still in NXP downstream repo.
>>>> Which DRM memory manager does that driver use?
>>> DRM_GEM_DMA_DRIVER_OPS
>> So fbdev-dma would be correct. But with the gem-dma allocator, wouldn't you
>> allocate from one of these dma_alloc_() calls at [1] ? How does the drivers
>> end up with vmalloc'd fbdev memory? Specifically in the light of the docs at
>> [2].
> I think when you have an iommu dma_alloc just allocates pages, and uses
> the iommu to make it all contiguous for the device, and vmalloc for the
> kernel. So it's not allocated with vmalloc(), just ends up getting mapped
> into the vmalloc range.

Thanks, that makes sense. As I mentioned in another reply, it might make 
sense to protect smem_start with an opt-in flag for only that single 
driver that needs it.

Best regards
Thomas

> -Sima
>
>
>> [1] https://elixir.bootlin.com/linux/v6.9/source/drivers/gpu/drm/drm_gem_dma_helper.c#L146
>> [2] https://elixir.bootlin.com/linux/v6.9/source/Documentation/core-api/dma-api-howto.rst#L124
>>
>> Best regards
>> Thomas
>>
>>>> I'm asking because if the driver allocates BOs via vmalloc(), should it really
>>>> use fbdev-dma?
>>> We use it for fbdev emulation. If drm_kms_helper.fbdev_emulation is 0,
>>> fbdev emulation could be disabled.
>>>
>>> Thanks
>>> Peng
>>>> Best regards
>>>> Thomas
>>>>
>>>>> Thanks,
>>>>> Peng.
>>>> --
>>>> --
>>>> Thomas Zimmermann
>>>> Graphics Driver Developer
>>>> SUSE Software Solutions Germany GmbH
>>>> Frankenstrasse 146, 90461 Nuernberg, Germany
>>>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB
>>>> 36809 (AG Nuernberg)
>> -- 
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> HRB 36809 (AG Nuernberg)
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH] drm/fbdev-dma: fix getting smem_start

[Thomas Zimmermann]

Hi

Am 10.06.24 um 10:47 schrieb Thomas Zimmermann:
> Hi
>
> Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
>> From: Peng Fan <peng.fan@nxp.com>
>>
>> If 'info->screen_buffer' locates in vmalloc address space, virt_to_page
>> will not be able to get correct results. With CONFIG_DEBUG_VM and
>> CONFIG_DEBUG_VIRTUAL enabled on ARM64, there is dump below:
>
> Which graphics driver triggers this bug?
>
>> [    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
>>
>> So add a check 'is_vmalloc_addr'.
>>
>> Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for 
>> GEM DMA helpers")
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

I'm taking back my r-b. The memory is expected to by be physically 
contiguous and vmalloc() won't guarantee that.

Best regards
Thomas

>
> Best regards
> Thomas
>
>> ---
>>   drivers/gpu/drm/drm_fbdev_dma.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fbdev_dma.c 
>> b/drivers/gpu/drm/drm_fbdev_dma.c
>> index 6c9427bb4053..9e2eddb6eb5c 100644
>> --- a/drivers/gpu/drm/drm_fbdev_dma.c
>> +++ b/drivers/gpu/drm/drm_fbdev_dma.c
>> @@ -130,7 +130,12 @@ 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 (is_vmalloc_addr(info->screen_buffer))
>> +        info->fix.smem_start = 
>> page_to_phys(vmalloc_to_page(info->screen_buffer));
>> +    else
>> +        info->fix.smem_start = 
>> page_to_phys(virt_to_page(info->screen_buffer));
>> +
>>       info->fix.smem_len = info->screen_size;
>>         return 0;
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH] drm/fbdev-dma: fix getting smem_start

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

> Hi
>
> Am 10.06.24 um 10:47 schrieb Thomas Zimmermann:
>> Hi
>>
>> Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
>>> From: Peng Fan <peng.fan@nxp.com>
>>>
>>> If 'info->screen_buffer' locates in vmalloc address space, virt_to_page
>>> will not be able to get correct results. With CONFIG_DEBUG_VM and
>>> CONFIG_DEBUG_VIRTUAL enabled on ARM64, there is dump below:
>>
>> Which graphics driver triggers this bug?
>>
>>> [    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
>>>
>>> So add a check 'is_vmalloc_addr'.
>>>
>>> Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for 
>>> GEM DMA helpers")
>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>
>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> I'm taking back my r-b. The memory is expected to by be physically 
> contiguous and vmalloc() won't guarantee that.
>

Agreed.

> Best regards
> Thomas
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH] drm/fbdev-dma: fix getting smem_start

[Thomas Zimmermann]

Hi Javier

Am 12.06.24 um 09:49 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
>
> Hello Thomas,
>
>> Hi
>>
>> Am 10.06.24 um 10:47 schrieb Thomas Zimmermann:
>>> Hi
>>>
>>> Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
>>>> From: Peng Fan <peng.fan@nxp.com>
>>>>
>>>> If 'info->screen_buffer' locates in vmalloc address space, virt_to_page
>>>> will not be able to get correct results. With CONFIG_DEBUG_VM and
>>>> CONFIG_DEBUG_VIRTUAL enabled on ARM64, there is dump below:
>>> Which graphics driver triggers this bug?
>>>
>>>> [    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
>>>>
>>>> So add a check 'is_vmalloc_addr'.
>>>>
>>>> Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for
>>>> GEM DMA helpers")
>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>> I'm taking back my r-b. The memory is expected to by be physically
>> contiguous and vmalloc() won't guarantee that.
>>
> Agreed.

These smem_ fields are clearly designed for PCI BARs of traditional 
graphics cards. So can we even assume contiguous memory for DMA? That 
was my assumption, but with IOMMUs it might not be the case. Fbdev-dma 
only sets smem_start to support a single old userspace driver. Maybe we 
should further restrict usage of this field by making it opt-in for each 
driver. Best regards Thomas
>
>> Best regards
>> Thomas
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH] drm/fbdev-dma: fix getting smem_start

[Daniel Vetter]

On Wed, Jun 12, 2024 at 10:37:14AM +0200, Thomas Zimmermann wrote:
> Hi Javier
> 
> Am 12.06.24 um 09:49 schrieb Javier Martinez Canillas:
> > Thomas Zimmermann <tzimmermann@suse.de> writes:
> > 
> > Hello Thomas,
> > 
> > > Hi
> > > 
> > > Am 10.06.24 um 10:47 schrieb Thomas Zimmermann:
> > > > Hi
> > > > 
> > > > Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > 
> > > > > If 'info->screen_buffer' locates in vmalloc address space, virt_to_page
> > > > > will not be able to get correct results. With CONFIG_DEBUG_VM and
> > > > > CONFIG_DEBUG_VIRTUAL enabled on ARM64, there is dump below:
> > > > Which graphics driver triggers this bug?
> > > > 
> > > > > [    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
> > > > > 
> > > > > So add a check 'is_vmalloc_addr'.
> > > > > 
> > > > > Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for
> > > > > GEM DMA helpers")
> > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > I'm taking back my r-b. The memory is expected to by be physically
> > > contiguous and vmalloc() won't guarantee that.
> > > 
> > Agreed.
> 
> These smem_ fields are clearly designed for PCI BARs of traditional graphics
> cards. So can we even assume contiguous memory for DMA? That was my
> assumption, but with IOMMUs it might not be the case. Fbdev-dma only sets
> smem_start to support a single old userspace driver. Maybe we should further
> restrict usage of this field by making it opt-in for each driver. Best
> regards Thomas

We could make it all conditional on CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM, and
remove the FBINFO_HIDE_SMEM_START flag. The reason I've done the flag is
that with the old fb_mmap code we had to always fill out smem_start to
make mmap work. But now that the various drm fbdev helpers have all their
own mmap implementation, we could make this a lot cleaner.

If I haven't missed anything, that is.
-Sima
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm/fbdev-dma: fix getting smem_start

[Thomas Zimmermann]

Hi

Am 12.06.24 um 17:00 schrieb Daniel Vetter:
> On Wed, Jun 12, 2024 at 10:37:14AM +0200, Thomas Zimmermann wrote:
>> Hi Javier
>>
>> Am 12.06.24 um 09:49 schrieb Javier Martinez Canillas:
>>> Thomas Zimmermann <tzimmermann@suse.de> writes:
>>>
>>> Hello Thomas,
>>>
>>>> Hi
>>>>
>>>> Am 10.06.24 um 10:47 schrieb Thomas Zimmermann:
>>>>> Hi
>>>>>
>>>>> Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
>>>>>> From: Peng Fan <peng.fan@nxp.com>
>>>>>>
>>>>>> If 'info->screen_buffer' locates in vmalloc address space, virt_to_page
>>>>>> will not be able to get correct results. With CONFIG_DEBUG_VM and
>>>>>> CONFIG_DEBUG_VIRTUAL enabled on ARM64, there is dump below:
>>>>> Which graphics driver triggers this bug?
>>>>>
>>>>>> [    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
>>>>>>
>>>>>> So add a check 'is_vmalloc_addr'.
>>>>>>
>>>>>> Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for
>>>>>> GEM DMA helpers")
>>>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> I'm taking back my r-b. The memory is expected to by be physically
>>>> contiguous and vmalloc() won't guarantee that.
>>>>
>>> Agreed.
>> These smem_ fields are clearly designed for PCI BARs of traditional graphics
>> cards. So can we even assume contiguous memory for DMA? That was my
>> assumption, but with IOMMUs it might not be the case. Fbdev-dma only sets
>> smem_start to support a single old userspace driver. Maybe we should further
>> restrict usage of this field by making it opt-in for each driver. Best
>> regards Thomas
> We could make it all conditional on CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM, and
> remove the FBINFO_HIDE_SMEM_START flag. The reason I've done the flag is
> that with the old fb_mmap code we had to always fill out smem_start to
> make mmap work. But now that the various drm fbdev helpers have all their
> own mmap implementation, we could make this a lot cleaner.

Enabling CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM would still crash the NXP 
driver. I think I'll add a flag to drm_fbdev_dma_setup() to set 
smem_start from within lima, which is the only driver that requires 
it.I'd like to remove CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM and all that, but 
I fear that it would break someone's setup. Best regards Thomas
>
> If I haven't missed anything, that is.
> -Sima

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH] drm/fbdev-dma: fix getting smem_start

[Thomas Zimmermann]

Hi

Am 13.06.24 um 12:14 schrieb Thomas Zimmermann:
> Hi
>
> Am 12.06.24 um 17:00 schrieb Daniel Vetter:
>> On Wed, Jun 12, 2024 at 10:37:14AM +0200, Thomas Zimmermann wrote:
>>> Hi Javier
>>>
>>> Am 12.06.24 um 09:49 schrieb Javier Martinez Canillas:
>>>> Thomas Zimmermann <tzimmermann@suse.de> writes:
>>>>
>>>> Hello Thomas,
>>>>
>>>>> Hi
>>>>>
>>>>> Am 10.06.24 um 10:47 schrieb Thomas Zimmermann:
>>>>>> Hi
>>>>>>
>>>>>> Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
>>>>>>> From: Peng Fan <peng.fan@nxp.com>
>>>>>>>
>>>>>>> If 'info->screen_buffer' locates in vmalloc address space, 
>>>>>>> virt_to_page
>>>>>>> will not be able to get correct results. With CONFIG_DEBUG_VM and
>>>>>>> CONFIG_DEBUG_VIRTUAL enabled on ARM64, there is dump below:
>>>>>> Which graphics driver triggers this bug?
>>>>>>
>>>>>>> [    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
>>>>>>>
>>>>>>> So add a check 'is_vmalloc_addr'.
>>>>>>>
>>>>>>> Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for
>>>>>>> GEM DMA helpers")
>>>>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>>>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>>> I'm taking back my r-b. The memory is expected to by be physically
>>>>> contiguous and vmalloc() won't guarantee that.
>>>>>
>>>> Agreed.
>>> These smem_ fields are clearly designed for PCI BARs of traditional 
>>> graphics
>>> cards. So can we even assume contiguous memory for DMA? That was my
>>> assumption, but with IOMMUs it might not be the case. Fbdev-dma only 
>>> sets
>>> smem_start to support a single old userspace driver. Maybe we should 
>>> further
>>> restrict usage of this field by making it opt-in for each driver. Best
>>> regards Thomas
>> We could make it all conditional on CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM, and
>> remove the FBINFO_HIDE_SMEM_START flag. The reason I've done the flag is
>> that with the old fb_mmap code we had to always fill out smem_start to
>> make mmap work. But now that the various drm fbdev helpers have all 
>> their
>> own mmap implementation, we could make this a lot cleaner.
>
> Enabling CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM would still crash the NXP 
> driver. I think I'll add a flag to drm_fbdev_dma_setup() to set 
> smem_start from within lima, which is the only driver that requires 
> it.I'd like to remove CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM and all that, 
> but I fear that it would break someone's setup. Best regards Thomas

I've been looking at

https://lore.kernel.org/dri-devel/20240318-dark-mongoose-of-camouflage-7ac6ed@houat/

and I'm now confused to find that lima doesn't even set up fbdev support.


Best regards
Thomas

>>
>> If I haven't missed anything, that is.
>> -Sima
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH] drm/fbdev-dma: fix getting smem_start

[Daniel Vetter]

On Thu, Jun 13, 2024 at 12:18:55PM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 13.06.24 um 12:14 schrieb Thomas Zimmermann:
> > Hi
> > 
> > Am 12.06.24 um 17:00 schrieb Daniel Vetter:
> > > On Wed, Jun 12, 2024 at 10:37:14AM +0200, Thomas Zimmermann wrote:
> > > > Hi Javier
> > > > 
> > > > Am 12.06.24 um 09:49 schrieb Javier Martinez Canillas:
> > > > > Thomas Zimmermann <tzimmermann@suse.de> writes:
> > > > > 
> > > > > Hello Thomas,
> > > > > 
> > > > > > Hi
> > > > > > 
> > > > > > Am 10.06.24 um 10:47 schrieb Thomas Zimmermann:
> > > > > > > Hi
> > > > > > > 
> > > > > > > Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
> > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > > 
> > > > > > > > If 'info->screen_buffer' locates in vmalloc
> > > > > > > > address space, virt_to_page
> > > > > > > > will not be able to get correct results. With CONFIG_DEBUG_VM and
> > > > > > > > CONFIG_DEBUG_VIRTUAL enabled on ARM64, there is dump below:
> > > > > > > Which graphics driver triggers this bug?
> > > > > > > 
> > > > > > > > [    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
> > > > > > > > 
> > > > > > > > So add a check 'is_vmalloc_addr'.
> > > > > > > > 
> > > > > > > > Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for
> > > > > > > > GEM DMA helpers")
> > > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > > > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > > > > I'm taking back my r-b. The memory is expected to by be physically
> > > > > > contiguous and vmalloc() won't guarantee that.
> > > > > > 
> > > > > Agreed.
> > > > These smem_ fields are clearly designed for PCI BARs of
> > > > traditional graphics
> > > > cards. So can we even assume contiguous memory for DMA? That was my
> > > > assumption, but with IOMMUs it might not be the case. Fbdev-dma
> > > > only sets
> > > > smem_start to support a single old userspace driver. Maybe we
> > > > should further
> > > > restrict usage of this field by making it opt-in for each driver. Best
> > > > regards Thomas
> > > We could make it all conditional on CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM, and
> > > remove the FBINFO_HIDE_SMEM_START flag. The reason I've done the flag is
> > > that with the old fb_mmap code we had to always fill out smem_start to
> > > make mmap work. But now that the various drm fbdev helpers have all
> > > their
> > > own mmap implementation, we could make this a lot cleaner.
> > 
> > Enabling CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM would still crash the NXP
> > driver. I think I'll add a flag to drm_fbdev_dma_setup() to set
> > smem_start from within lima, which is the only driver that requires
> > it.I'd like to remove CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM and all that, but
> > I fear that it would break someone's setup. Best regards Thomas

Yeah we'd always need to make this conditional on the memory not being in
the vmalloc range, or things will blow up.

> I've been looking at
> 
> https://lore.kernel.org/dri-devel/20240318-dark-mongoose-of-camouflage-7ac6ed@houat/
> 
> and I'm now confused to find that lima doesn't even set up fbdev support.

The mali driver here was the out-of-tree proprietary mali driver as the
consumer of such buffers.

The "exporters" was just any random fbdev driver, and with the DRM option
to set the smem, also drm drivers could play in this role. It at least
seems to have helped a few people to move away from out-of-tree fbdev
drivers to upstream drm drivers (but still with the out-of-tree mali gpu
driver). Which means we've needed this for any display driver that
happens to have shipped together with one of these older mali gpus.

It's a bit a mess, and it might indeed have outlived it's usefulness.

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

Re: [PATCH] drm/fbdev-dma: fix getting smem_start

[Thomas Zimmermann]

Hi

Am 17.06.24 um 16:04 schrieb Daniel Vetter:
> On Thu, Jun 13, 2024 at 12:18:55PM +0200, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 13.06.24 um 12:14 schrieb Thomas Zimmermann:
>>> Hi
>>>
>>> Am 12.06.24 um 17:00 schrieb Daniel Vetter:
>>>> On Wed, Jun 12, 2024 at 10:37:14AM +0200, Thomas Zimmermann wrote:
>>>>> Hi Javier
>>>>>
>>>>> Am 12.06.24 um 09:49 schrieb Javier Martinez Canillas:
>>>>>> Thomas Zimmermann <tzimmermann@suse.de> writes:
>>>>>>
>>>>>> Hello Thomas,
>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> Am 10.06.24 um 10:47 schrieb Thomas Zimmermann:
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> Am 04.06.24 um 10:03 schrieb Peng Fan (OSS):
>>>>>>>>> From: Peng Fan <peng.fan@nxp.com>
>>>>>>>>>
>>>>>>>>> If 'info->screen_buffer' locates in vmalloc
>>>>>>>>> address space, virt_to_page
>>>>>>>>> will not be able to get correct results. With CONFIG_DEBUG_VM and
>>>>>>>>> CONFIG_DEBUG_VIRTUAL enabled on ARM64, there is dump below:
>>>>>>>> Which graphics driver triggers this bug?
>>>>>>>>
>>>>>>>>> [    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
>>>>>>>>>
>>>>>>>>> So add a check 'is_vmalloc_addr'.
>>>>>>>>>
>>>>>>>>> Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for
>>>>>>>>> GEM DMA helpers")
>>>>>>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>>>>>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>>>>> I'm taking back my r-b. The memory is expected to by be physically
>>>>>>> contiguous and vmalloc() won't guarantee that.
>>>>>>>
>>>>>> Agreed.
>>>>> These smem_ fields are clearly designed for PCI BARs of
>>>>> traditional graphics
>>>>> cards. So can we even assume contiguous memory for DMA? That was my
>>>>> assumption, but with IOMMUs it might not be the case. Fbdev-dma
>>>>> only sets
>>>>> smem_start to support a single old userspace driver. Maybe we
>>>>> should further
>>>>> restrict usage of this field by making it opt-in for each driver. Best
>>>>> regards Thomas
>>>> We could make it all conditional on CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM, and
>>>> remove the FBINFO_HIDE_SMEM_START flag. The reason I've done the flag is
>>>> that with the old fb_mmap code we had to always fill out smem_start to
>>>> make mmap work. But now that the various drm fbdev helpers have all
>>>> their
>>>> own mmap implementation, we could make this a lot cleaner.
>>> Enabling CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM would still crash the NXP
>>> driver. I think I'll add a flag to drm_fbdev_dma_setup() to set
>>> smem_start from within lima, which is the only driver that requires
>>> it.I'd like to remove CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM and all that, but
>>> I fear that it would break someone's setup. Best regards Thomas
> Yeah we'd always need to make this conditional on the memory not being in
> the vmalloc range, or things will blow up.
>
>> I've been looking at
>>
>> https://lore.kernel.org/dri-devel/20240318-dark-mongoose-of-camouflage-7ac6ed@houat/
>>
>> and I'm now confused to find that lima doesn't even set up fbdev support.
> The mali driver here was the out-of-tree proprietary mali driver as the
> consumer of such buffers.
>
> The "exporters" was just any random fbdev driver, and with the DRM option
> to set the smem, also drm drivers could play in this role. It at least
> seems to have helped a few people to move away from out-of-tree fbdev
> drivers to upstream drm drivers (but still with the out-of-tree mali gpu
> driver). Which means we've needed this for any display driver that
> happens to have shipped together with one of these older mali gpus.
>
> It's a bit a mess, and it might indeed have outlived it's usefulness.

Thanks a lot for explaining. Sounds like the kind of workaround that is 
almost impossible to remove.

Let me send out a patch that connects the setting in fbdev-dma to the 
drm_leak_fbdev_smem parameter. At least fbdev-dma won't try to set 
smem_start unnecessarily.


Best regards
Thomas

>
> Cheers, Sima

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)