[PATCH] drm/fb-helper: Fix out-of-bounds access

[PATCH] drm/fb-helper: Fix out-of-bounds access

[Thomas Zimmermann]

Clip memory range to screen-buffer size to avoid out-of-bounds access
in fbdev deferred I/O's damage handling.

Fbdev's deferred I/O can only track pages. From the range of pages, the
damage handler computes the clipping rectangle for the display update.
If the fbdev screen buffer ends near the beginning of a page, that page
could contain more scanlines. The damage handler would then track these
non-existing scanlines as dirty and provoke an out-of-bounds access
during the screen update. Hence, clip the maximum memory range to the
size of the screen buffer.

While at it, rename the variables min/max to min_off/max_off in
drm_fb_helper_deferred_io(). This avoids confusion with the macros of
the same name.

Reported-by: Nuno Gonçalves <nunojpg@gmail.com>
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Nuno Gonçalves <nunojpg@gmail.com>
Fixes: 67b723f5b742 ("drm/fb-helper: Calculate damaged area in separate helper")
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: <stable@vger.kernel.org> # v5.18+
---
 drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 5ad2b6a2778c..1705e8d345ab 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -680,7 +680,11 @@ static void drm_fb_helper_damage(struct fb_info *info, u32 x, u32 y,
 	schedule_work(&helper->damage_work);
 }
 
-/* Convert memory region into area of scanlines and pixels per scanline */
+/*
+ * Convert memory region into area of scanlines and pixels per
+ * scanline. The parameters off and len must not reach beyond
+ * the end of the framebuffer.
+ */
 static void drm_fb_helper_memory_range_to_clip(struct fb_info *info, off_t off, size_t len,
 					       struct drm_rect *clip)
 {
@@ -715,22 +719,29 @@ static void drm_fb_helper_memory_range_to_clip(struct fb_info *info, off_t off,
  */
 void drm_fb_helper_deferred_io(struct fb_info *info, struct list_head *pagereflist)
 {
-	unsigned long start, end, min, max;
+	unsigned long start, end, min_off, max_off;
 	struct fb_deferred_io_pageref *pageref;
 	struct drm_rect damage_area;
 
-	min = ULONG_MAX;
-	max = 0;
+	min_off = ULONG_MAX;
+	max_off = 0;
 	list_for_each_entry(pageref, pagereflist, list) {
 		start = pageref->offset;
 		end = start + PAGE_SIZE;
-		min = min(min, start);
-		max = max(max, end);
+		min_off = min(min_off, start);
+		max_off = max(max_off, end);
 	}
-	if (min >= max)
+	if (min_off >= max_off)
 		return;
 
-	drm_fb_helper_memory_range_to_clip(info, min, max - min, &damage_area);
+	/*
+	 * As we can only track pages, we might reach beyond the end
+	 * of the screen and account for non-existing scanlines. Hence,
+	 * keep the covered memory area within the screen buffer.
+	 */
+	max_off = min(max_off, info->screen_size);
+
+	drm_fb_helper_memory_range_to_clip(info, min_off, max_off - min_off, &damage_area);
 	drm_fb_helper_damage(info, damage_area.x1, damage_area.y1,
 			     drm_rect_width(&damage_area),
 			     drm_rect_height(&damage_area));
-- 
2.36.1

Re: [PATCH] drm/fb-helper: Fix out-of-bounds access

[Javier Martinez Canillas]

Hello Thomas,

On 6/21/22 12:46, Thomas Zimmermann wrote:
> Clip memory range to screen-buffer size to avoid out-of-bounds access
> in fbdev deferred I/O's damage handling.
> 
> Fbdev's deferred I/O can only track pages. From the range of pages, the
> damage handler computes the clipping rectangle for the display update.
> If the fbdev screen buffer ends near the beginning of a page, that page
> could contain more scanlines. The damage handler would then track these
> non-existing scanlines as dirty and provoke an out-of-bounds access
> during the screen update. Hence, clip the maximum memory range to the
> size of the screen buffer.
> 
> While at it, rename the variables min/max to min_off/max_off in
> drm_fb_helper_deferred_io(). This avoids confusion with the macros of
> the same name.
> 
> Reported-by: Nuno Gonçalves <nunojpg@gmail.com>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Tested-by: Nuno Gonçalves <nunojpg@gmail.com>
> Fixes: 67b723f5b742 ("drm/fb-helper: Calculate damaged area in separate helper")
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: <stable@vger.kernel.org> # v5.18+
> ---

This makes sense to me.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH] drm/fb-helper: Fix out-of-bounds access

[Geert Uytterhoeven]

[-- Attachment #1: Type: text/plain, Size: 2046 bytes --]

 	Hi Thomas,

On Tue, 21 Jun 2022, Thomas Zimmermann wrote:
> Clip memory range to screen-buffer size to avoid out-of-bounds access
> in fbdev deferred I/O's damage handling.
>
> Fbdev's deferred I/O can only track pages. From the range of pages, the
> damage handler computes the clipping rectangle for the display update.
> If the fbdev screen buffer ends near the beginning of a page, that page
> could contain more scanlines. The damage handler would then track these
> non-existing scanlines as dirty and provoke an out-of-bounds access
> during the screen update. Hence, clip the maximum memory range to the
> size of the screen buffer.
>
> While at it, rename the variables min/max to min_off/max_off in
> drm_fb_helper_deferred_io(). This avoids confusion with the macros of
> the same name.
>
> Reported-by: Nuno Gonçalves <nunojpg@gmail.com>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Tested-by: Nuno Gonçalves <nunojpg@gmail.com>
> Fixes: 67b723f5b742 ("drm/fb-helper: Calculate damaged area in separate helper")

Thanks for your patch, which is now commit ae25885bdf59fde4
("drm/fb-helper: Fix out-of-bounds access") in drm-misc/for-linux-next.

I had seen the crash before, but thought it was a bug in my wip
atari-drm driver.  When diving deeper today, and consequently looking
for recent changes to the damage helper, I found this commit in
linux-next.

With your patch instead of my own workaround I used this morning, [1]
still works fine, so:
Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>.
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>.

[1] [PATCH] drm/fb-helper: Remove helpers to change frame buffer config
     https://lore.kernel.org/all/20220629105658.1373770-1-geert@linux-m68k.org

Gr{oetje,eeting}s,

 						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
 							    -- Linus Torvalds

Re: [PATCH] drm/fb-helper: Fix out-of-bounds access

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 2642 bytes --]

Hi

Am 29.06.22 um 20:17 schrieb Geert Uytterhoeven:
>      Hi Thomas,
> 
> On Tue, 21 Jun 2022, Thomas Zimmermann wrote:
>> Clip memory range to screen-buffer size to avoid out-of-bounds access
>> in fbdev deferred I/O's damage handling.
>>
>> Fbdev's deferred I/O can only track pages. From the range of pages, the
>> damage handler computes the clipping rectangle for the display update.
>> If the fbdev screen buffer ends near the beginning of a page, that page
>> could contain more scanlines. The damage handler would then track these
>> non-existing scanlines as dirty and provoke an out-of-bounds access
>> during the screen update. Hence, clip the maximum memory range to the
>> size of the screen buffer.
>>
>> While at it, rename the variables min/max to min_off/max_off in
>> drm_fb_helper_deferred_io(). This avoids confusion with the macros of
>> the same name.
>>
>> Reported-by: Nuno Gonçalves <nunojpg@gmail.com>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Tested-by: Nuno Gonçalves <nunojpg@gmail.com>
>> Fixes: 67b723f5b742 ("drm/fb-helper: Calculate damaged area in 
>> separate helper")
> 
> Thanks for your patch, which is now commit ae25885bdf59fde4
> ("drm/fb-helper: Fix out-of-bounds access") in drm-misc/for-linux-next.
> 
> I had seen the crash before, but thought it was a bug in my wip
> atari-drm driver.  When diving deeper today, and consequently looking
> for recent changes to the damage helper, I found this commit in
> linux-next.
> 
> With your patch instead of my own workaround I used this morning, [1]
> still works fine, so:
> Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>.
> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>.

Great thanks a lot.

BTW, what's the status of the atari-drm driver?

Best regard
Thomas

> 
> [1] [PATCH] drm/fb-helper: Remove helpers to change frame buffer config
>      
> https://lore.kernel.org/all/20220629105658.1373770-1-geert@linux-m68k.org
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. 
> But
> when I'm talking to journalists I just say "programmer" or something 
> like that.
>                                  -- Linus Torvalds

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

drm/fbdev-dma: regression

[Nuno Gonçalves]

Hi Thomas,

Since 5ab91447aa13b8b98bc11f5326f33500b0ee2c48 and still happening in
master, I often get a kernel crash, either a "Unable to handle kernel
NULL pointer dereference at virtual address" or "Unable to handle
kernel paging request at virtual address".

This happens in ARM64 with ili9225 and display size 220, 176. It also
happens in another board with a different tinydrm driver and size 320,
240.

I've reported in 2022 a bug (that you fixed, thanks!), where it was
related to screen size causing non alignment of scanlines and pages.
I've not investigated further this time as I was not doing any driver
development, but let me know if I can do any further testing.

Thanks,
Nuno

Re: drm/fbdev-dma: regression

[Thorsten Leemhuis]

[CCing a few more lists]

On 21.10.24 15:03, Nuno Gonçalves wrote:
> 
> Since 5ab91447aa13b8b98bc11f5326f33500b0ee2c48 and still happening in
> master, I often get a kernel crash, either a "Unable to handle kernel
> NULL pointer dereference at virtual address" or "Unable to handle
> kernel paging request at virtual address".
> 
> This happens in ARM64 with ili9225 and display size 220, 176. It also
> happens in another board with a different tinydrm driver and size 320,
> 240.
> 
> I've reported in 2022 a bug (that you fixed, thanks!), where it was
> related to screen size causing non alignment of scanlines and pages.
> I've not investigated further this time as I was not doing any driver
> development, but let me know if I can do any further testing.

Thx for the report. I might be missing something, but from here it looks
like nothing happened. So allow me to ask:

What's the status? Did anyone look into this? Is this sill happening?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

Re: drm/fbdev-dma: regression

[Thomas Zimmermann]

Hi


Am 11.11.24 um 12:51 schrieb Thorsten Leemhuis:
> [CCing a few more lists]
>
> On 21.10.24 15:03, Nuno Gonçalves wrote:
>> Since 5ab91447aa13b8b98bc11f5326f33500b0ee2c48 and still happening in
>> master, I often get a kernel crash, either a "Unable to handle kernel
>> NULL pointer dereference at virtual address" or "Unable to handle
>> kernel paging request at virtual address".
>>
>> This happens in ARM64 with ili9225 and display size 220, 176. It also
>> happens in another board with a different tinydrm driver and size 320,
>> 240.
>>
>> I've reported in 2022 a bug (that you fixed, thanks!), where it was
>> related to screen size causing non alignment of scanlines and pages.
>> I've not investigated further this time as I was not doing any driver
>> development, but let me know if I can do any further testing.
> Thx for the report. I might be missing something, but from here it looks
> like nothing happened. So allow me to ask:
>
> What's the status? Did anyone look into this? Is this sill happening?

I briefly looked into this bug report some time ago.

The patch in question changes the whole memory management of the 
affected code. It's also noteworthy that most of it has been reworked 
for the upcoming v6.12. Maybe this already fixed the problem. Kernel 
v6.11-rc7 added commit 5a498d4d06d6 ("drm/fbdev-dma: Only install 
deferred I/O if necessary"), which possibly fixes the problem as well.

But there's no explicit fix for this problem and I have not seen any 
other related reports. Any further information is welcome.

Best regards
Thomas

>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> #regzbot poke

-- 
--
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: drm/fbdev-dma: regression

[Nuno Gonçalves]

On Mon, Nov 11, 2024 at 1:22 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> The patch in question changes the whole memory management of the
> affected code. It's also noteworthy that most of it has been reworked
> for the upcoming v6.12. Maybe this already fixed the problem. Kernel
> v6.11-rc7 added commit 5a498d4d06d6 ("drm/fbdev-dma: Only install
> deferred I/O if necessary"), which possibly fixes the problem as well.
>
> But there's no explicit fix for this problem and I have not seen any
> other related reports. Any further information is welcome.

Issue was present since 5ab91447aa13b8b98bc11f5326f33500b0ee2c48 and
tested until 6.12-rc3.
Is there any suggestion on how to dig down?

Thanks

Re: drm/fbdev-dma: regression

[Thomas Zimmermann]

Hi


Am 11.11.24 um 14:42 schrieb Nuno Gonçalves:
> On Mon, Nov 11, 2024 at 1:22 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> The patch in question changes the whole memory management of the
>> affected code. It's also noteworthy that most of it has been reworked
>> for the upcoming v6.12. Maybe this already fixed the problem. Kernel
>> v6.11-rc7 added commit 5a498d4d06d6 ("drm/fbdev-dma: Only install
>> deferred I/O if necessary"), which possibly fixes the problem as well.
>>
>> But there's no explicit fix for this problem and I have not seen any
>> other related reports. Any further information is welcome.
> Issue was present since 5ab91447aa13b8b98bc11f5326f33500b0ee2c48 and
> tested until 6.12-rc3.
> Is there any suggestion on how to dig down?

Do you have a stack trace of this problem?

Which hardware platform is affected?

Best regards
Thomas

>
> Thanks

-- 
--
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: drm/fbdev-dma: regression

[Nuno Gonçalves]

[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

On Mon, Nov 11, 2024, 14:37 Thomas Zimmermann <tzimmermann@suse.de> wrote:

> Hi
>
>
> Am 11.11.24 um 14:42 schrieb Nuno Gonçalves:
> > On Mon, Nov 11, 2024 at 1:22 PM Thomas Zimmermann <tzimmermann@suse.de>
> wrote:
> >> The patch in question changes the whole memory management of the
> >> affected code. It's also noteworthy that most of it has been reworked
> >> for the upcoming v6.12. Maybe this already fixed the problem. Kernel
> >> v6.11-rc7 added commit 5a498d4d06d6 ("drm/fbdev-dma: Only install
> >> deferred I/O if necessary"), which possibly fixes the problem as well.
> >>
> >> But there's no explicit fix for this problem and I have not seen any
> >> other related reports. Any further information is welcome.
> > Issue was present since 5ab91447aa13b8b98bc11f5326f33500b0ee2c48 and
> > tested until 6.12-rc3.
> > Is there any suggestion on how to dig down?
>
> Do you have a stack trace of this problem?
>

No. I need to build kernel with debug?



> Which hardware platform is affected?
>

ARM64 with ili9225 and display size 220, 176. It also
happens in another board with a different tinydrm driver and size 320,
240.

[-- Attachment #2: Type: text/html, Size: 1997 bytes --]

Re: drm/fbdev-dma: regression

[Thomas Zimmermann]

Hi


Am 11.11.24 um 20:06 schrieb Nuno Gonçalves:
>
>
> On Mon, Nov 11, 2024, 14:37 Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
>     Hi
>
>
>     Am 11.11.24 um 14:42 schrieb Nuno Gonçalves:
>     > On Mon, Nov 11, 2024 at 1:22 PM Thomas Zimmermann
>     <tzimmermann@suse.de> wrote:
>     >> The patch in question changes the whole memory management of the
>     >> affected code. It's also noteworthy that most of it has been
>     reworked
>     >> for the upcoming v6.12. Maybe this already fixed the problem.
>     Kernel
>     >> v6.11-rc7 added commit 5a498d4d06d6 ("drm/fbdev-dma: Only install
>     >> deferred I/O if necessary"), which possibly fixes the problem
>     as well.
>     >>
>     >> But there's no explicit fix for this problem and I have not
>     seen any
>     >> other related reports. Any further information is welcome.
>     > Issue was present since 5ab91447aa13b8b98bc11f5326f33500b0ee2c48 and
>     > tested until 6.12-rc3.
>     > Is there any suggestion on how to dig down?
>
>     Do you have a stack trace of this problem?
>
>
> No. I need to build kernel with debug?

Isn't there an error message in the output of dmesg?

Or maybe can you save the whole output of dmesg after a crash and attach 
the file to an email?

In the original bug report, you mentioned that you often get an error. 
But not 'always'?

Do you have a reproducer for the problem? Such as a very simple program 
that triggers the bug.

>
>
>
>     Which hardware platform is affected?
>
>
> ARM64 with ili9225 and display size 220, 176. It also
> happens in another board with a different tinydrm driver and size 320,
> 240.

I meant, which board or CPU. ARM64/Aarch64 is really just a very broad 
term describing the system.

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: drm/fbdev-dma: regression

[Thomas Zimmermann]

[-- Attachment #1: Type: text/plain, Size: 1219 bytes --]

Hi

Am 11.11.24 um 14:42 schrieb Nuno Gonçalves:
> On Mon, Nov 11, 2024 at 1:22 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> The patch in question changes the whole memory management of the
>> affected code. It's also noteworthy that most of it has been reworked
>> for the upcoming v6.12. Maybe this already fixed the problem. Kernel
>> v6.11-rc7 added commit 5a498d4d06d6 ("drm/fbdev-dma: Only install
>> deferred I/O if necessary"), which possibly fixes the problem as well.
>>
>> But there's no explicit fix for this problem and I have not seen any
>> other related reports. Any further information is welcome.
> Issue was present since 5ab91447aa13b8b98bc11f5326f33500b0ee2c48 and
> tested until 6.12-rc3.
> Is there any suggestion on how to dig down?

Here's a first attempt to address this bug. Could you please apply the 
attached patch and report on the results? It should work against the 
upcoming v6.13-rc1 or against a recent drm-misc-next.

Best regards
Thomas

>
> Thanks

-- 
--
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)

[-- Attachment #2: 0001-fbdev-dma-Support-deferred-I-O-without-smem_start.patch --]
[-- Type: text/x-patch, Size: 1886 bytes --]

From 40d481266497437ebaa5312c9af9641361b172f8 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Thu, 28 Nov 2024 14:25:03 +0100
Subject: [PATCH] fbdev-dma: Support deferred I/O without smem_start

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_fbdev_dma.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c
index b14b581c059d..6a94d1b62479 100644
--- a/drivers/gpu/drm/drm_fbdev_dma.c
+++ b/drivers/gpu/drm/drm_fbdev_dma.c
@@ -8,6 +8,7 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_dma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 
 /*
  * struct fb_ops
@@ -124,6 +125,28 @@ static const struct drm_fb_helper_funcs drm_fbdev_dma_helper_funcs = {
 	.fb_dirty = drm_fbdev_dma_helper_fb_dirty,
 };
 
+static struct page *drm_fbdev_dma_get_page(struct fb_info *info, unsigned long offset)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	struct drm_framebuffer *fb = fb_helper->fb;
+	struct drm_gem_object *obj = drm_gem_fb_get_obj(fb, 0);
+	const void *addr = info->screen_buffer + offset;
+	struct page *page = NULL;
+
+	if (fb_WARN_ON_ONCE(info, offset > obj->size))
+		return NULL;
+
+	if (is_vmalloc_addr(addr))
+		page = vmalloc_to_page(addr);
+	else if (virt_addr_valid(addr))
+		page = virt_to_page(addr);
+
+	if (!fb_WARN_ON_ONCE(info, !page))
+		get_page(page);
+
+	return page;
+}
+
 /*
  * struct drm_fb_helper
  */
@@ -217,6 +240,7 @@ int drm_fbdev_dma_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
 	/* deferred I/O */
 	if (use_deferred_io) {
 		fb_helper->fbdefio.delay = HZ / 20;
+		fb_helper->fbdefio.get_page = drm_fbdev_dma_get_page;
 		fb_helper->fbdefio.deferred_io = drm_fb_helper_deferred_io;
 
 		info->fbdefio = &fb_helper->fbdefio;
-- 
2.47.0

Re: drm/fbdev-dma: regression

[Nuno Gonçalves]

> Here's a first attempt to address this bug. Could you please apply the
> attached patch and report on the results? It should work against the
> upcoming v6.13-rc1 or against a recent drm-misc-next.

Hi. No luck yet.

I collected crashes before the patch:

BUG: Bad page map in process husband  pte:60000001b46fcb pmd:800000002736403
page: refcount:1 mapcount:-1 mapping:000000005cb4182c index:0x6 pfn:0x1b46
aops:0xffff8000807722e0 ino:15 dentry name(?):"fb0"
flags: 0x14(referenced|dirty|zone=0)
raw: 0000000000000014 dead000000000100 dead000000000122 ffff0000018cf348
raw: 0000000000000006 0000000000000000 00000001fffffffe 0000000000000000
page dumped because: bad pte
addr:0000ffff87b4e000 vm_flags:040400fb anon_vma:0000000000000000
mapping:ffff0000018cf348 index:6
file:fb0 fault:fb_deferred_io_fault mmap:fb_mmap read_folio:0x0
CPU: 2 UID: 0 PID: 313 Comm: husband Not tainted 6.12.0 #1
Hardware name: Raspberry Pi Compute Module 3 Rev 1.0 (DT)
Call trace:
 show_stack+0x18/0x30 (C)
 dump_stack_lvl+0x60/0x80
 dump_stack+0x18/0x24
 print_bad_pte+0x174/0x1d0
 unmap_page_range+0x47c/0xc90
 unmap_vmas+0xa4/0x100
 exit_mmap+0xbc/0x2c0
 mmput+0x70/0x190
 do_exit+0x248/0x8f0
 do_group_exit+0x34/0x90
 get_signal+0x834/0x860
 do_signal+0xf4/0x330
 do_notify_resume+0xc0/0x140
 el0_svc+0xb8/0xd0
 el0t_64_sync_handler+0x10c/0x140
 el0t_64_sync+0x198/0x19c
Disabling lock debugging due to kernel taint

And after the patch:

lk_smartMem abort info:
 /tmp/0c  ESR = 0x0000000096000044
8081f6-1  EC = 0x25: DABT (current EL), IL = 32 bits
165-4fd2  SET = 0, FnV = 0
-5e4e03b  EA = 0, S1PTW = 0
e-57fa8e  FSC = 0x04: level 0 translation fault
22.dmp
Data abort info:
  ISV = 0, ISS = 0x00000044, ISS2 = 0x00000000
  CM = 0, WnR = 1, TnD = 0, TagAccess = 0
  GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ffbf801f8e7bf010] address between user and kernel address ranges
Internal error: Oops: 0000000096000044 [#1] SMP
CPU: 1 UID: 0 PID: 260 Comm: husband Not tainted 6.12.0 #1
Hardware name: Raspberry Pi Compute Module 3 Rev 1.0 (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : tlb_remove_table+0xc8/0x1c0
lr : tlb_remove_table+0xc0/0x1c0
sp : ffff80008117b9d0
x29: ffff80008117b9d0 x28: ffff80008117bb98 x27: 0000aaaad1df8000
x26: 0000aaaad1df7fff x25: ffff0000044f4468 x24: 00000000ffffffdb
x23: fffffdffc008f2c0 x22: ffff8000808a1860 x21: 0000aaaad1c00000
x20: fffffdffc008f2c0 x19: ffff80008117bb98 x18: ffff80008117bb50
x17: ffffffffffffffff x16: 0000000000000000 x15: ffff8000807b62a8
x14: 0000ffffb7640fff x13: 0000000000000000 x12: 0000000000000000
x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000037b6f
x8 : ffff80008094a0e7 x7 : 0000000000000800 x6 : 0000000000000000
x5 : 0000000000000801 x4 : dead000000000122 x3 : 0000000000000000
x2 : 0000020040000000 x1 : ffff000000000000 x0 : ffbf801f8e7bf000
Call trace:
 tlb_remove_table+0xc8/0x1c0 (P)
 tlb_remove_table+0xc0/0x1c0 (L)
 free_pgd_range+0x228/0x5d0
 free_pgtables+0x1c4/0x270
 exit_mmap+0x130/0x2c0
 mmput+0x70/0x190
 do_exit+0x248/0x8f0
 do_group_exit+0x34/0x90
 get_signal+0x834/0x860
 do_signal+0x118/0x330
 do_notify_resume+0xc0/0x140
 el0_svc+0xb8/0xd0
 el0t_64_sync_handler+0x10c/0x140
 el0t_64_sync+0x198/0x19c
Code: 52850000 94006fdd f9000660 b4000080 (b900101f)
---[ end trace 0000000000000000 ]---
Fixing recursive fault but reboot is needed!
Unable to handle kernel NULL pointer dereference at virtual address
0000000000000008
Mem abort info:
  ESR = 0x0000000096000044
  EC = 0x25: DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
  FSC = 0x04: level 0 translation fault
Data abort info:
  ISV = 0, ISS = 0x00000044, ISS2 = 0x00000000
  CM = 0, WnR = 1, TnD = 0, TagAccess = 0
  GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
user pgtable: 4k pages, 48-bit VAs, pgdp=000000000238c000
[0000000000000008] pgd=0000000000000000, p4d=0000000000000000
Internal error: Oops: 0000000096000044 [#2] SMP
CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Tainted: G      D            6.12.0 #1
Tainted: [D]=DIE
Hardware name: Raspberry Pi Compute Module 3 Rev 1.0 (DT)
pstate: a0000005 (NzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : free_unref_page_commit+0x70/0x2c0
lr : free_unref_page+0x17c/0x400
sp : ffff800080003da0
x29: ffff800080003da0 x28: 0000000000000000 x27: 0000000000000000
x26: 0000000000000000 x25: ffff00003e39e0b8 x24: 0000000000000000
x23: 0000000000000000 x22: 00000000ffffffff x21: ffff00003e39efc0
x20: 0000000000000000 x19: ffff800080add340 x18: ffff800081163c40
x17: ffff7fffbda55000 x16: ffff800080000000 x15: 0000000000001900
x14: 00000f423fffe700 x13: 0000000000000000 x12: ffff00003e39efc0
x11: 0000000000000001 x10: 0000000000000001 x9 : ffff7fffbda55000
x8 : fffffdffc00e81c8 x7 : ffff00003e39efe0 x6 : 0000000000000020
x5 : ffff00003e39efc0 x4 : 00000000ffffffff x3 : 0000000000000000
x2 : fffffdffc00e81c0 x1 : 0000000000000000 x0 : ffff800080add340
Call trace:
 free_unref_page_commit+0x70/0x2c0 (P)
 free_unref_page+0x17c/0x400 (L)
 free_unref_page+0x17c/0x400
 __folio_put+0x50/0xb0
 tlb_remove_table_rcu+0xb0/0xc0
 rcu_core+0x1f4/0x520
 rcu_core_si+0x10/0x20
 handle_softirqs+0xf4/0x230
 __do_softirq+0x14/0x20
 ____do_softirq+0x10/0x20
 call_on_irq_stack+0x24/0x54
 do_softirq_own_stack+0x1c/0x30
 __irq_exit_rcu+0xc8/0x100
 irq_exit_rcu+0x10/0x20
 el1_interrupt+0x38/0x60
 el1h_64_irq_handler+0x18/0x30
 el1h_64_irq+0x6c/0x70
 default_idle_call+0x28/0x3c (P)
 default_idle_call+0x24/0x3c (L)
 do_idle+0x9c/0xf0
 cpu_startup_entry+0x34/0x40
 rest_init+0xb8/0xc0
 do_one_initcall+0x0/0x16c
 __primary_switched+0x88/0x90
Code: 8b2ac0e7 f8296827 8b0600a7 f9401181 (f9000428)
---[ end trace 0000000000000000 ]---
Kernel panic - not syncing: Oops: Fatal exception in interrupt
SMP: stopping secondary CPUs
Kernel Offset: disabled
CPU features: 0x000,00000c00,00800000,0200420b
Memory Limit: none
---[ end Kernel panic - not syncing: Oops: Fatal exception in interrupt ]---

Thanks,
Nuno

Re: drm/fbdev-dma: regression

[Thomas Zimmermann]

[-- Attachment #1: Type: text/plain, Size: 6731 bytes --]

Hi


Am 29.11.24 um 00:37 schrieb Nuno Gonçalves:
>> Here's a first attempt to address this bug. Could you please apply the
>> attached patch and report on the results? It should work against the
>> upcoming v6.13-rc1 or against a recent drm-misc-next.
> Hi. No luck yet.

Thanks for testing. Attached is another patch. It attempts to restore 
the fbdev handling as before the regression. Please give it a try.

Best regards
Thomas

>
> I collected crashes before the patch:
>
> BUG: Bad page map in process husband  pte:60000001b46fcb pmd:800000002736403
> page: refcount:1 mapcount:-1 mapping:000000005cb4182c index:0x6 pfn:0x1b46
> aops:0xffff8000807722e0 ino:15 dentry name(?):"fb0"
> flags: 0x14(referenced|dirty|zone=0)
> raw: 0000000000000014 dead000000000100 dead000000000122 ffff0000018cf348
> raw: 0000000000000006 0000000000000000 00000001fffffffe 0000000000000000
> page dumped because: bad pte
> addr:0000ffff87b4e000 vm_flags:040400fb anon_vma:0000000000000000
> mapping:ffff0000018cf348 index:6
> file:fb0 fault:fb_deferred_io_fault mmap:fb_mmap read_folio:0x0
> CPU: 2 UID: 0 PID: 313 Comm: husband Not tainted 6.12.0 #1
> Hardware name: Raspberry Pi Compute Module 3 Rev 1.0 (DT)
> Call trace:
>   show_stack+0x18/0x30 (C)
>   dump_stack_lvl+0x60/0x80
>   dump_stack+0x18/0x24
>   print_bad_pte+0x174/0x1d0
>   unmap_page_range+0x47c/0xc90
>   unmap_vmas+0xa4/0x100
>   exit_mmap+0xbc/0x2c0
>   mmput+0x70/0x190
>   do_exit+0x248/0x8f0
>   do_group_exit+0x34/0x90
>   get_signal+0x834/0x860
>   do_signal+0xf4/0x330
>   do_notify_resume+0xc0/0x140
>   el0_svc+0xb8/0xd0
>   el0t_64_sync_handler+0x10c/0x140
>   el0t_64_sync+0x198/0x19c
> Disabling lock debugging due to kernel taint
>
> And after the patch:
>
> lk_smartMem abort info:
>   /tmp/0c  ESR = 0x0000000096000044
> 8081f6-1  EC = 0x25: DABT (current EL), IL = 32 bits
> 165-4fd2  SET = 0, FnV = 0
> -5e4e03b  EA = 0, S1PTW = 0
> e-57fa8e  FSC = 0x04: level 0 translation fault
> 22.dmp
> Data abort info:
>    ISV = 0, ISS = 0x00000044, ISS2 = 0x00000000
>    CM = 0, WnR = 1, TnD = 0, TagAccess = 0
>    GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [ffbf801f8e7bf010] address between user and kernel address ranges
> Internal error: Oops: 0000000096000044 [#1] SMP
> CPU: 1 UID: 0 PID: 260 Comm: husband Not tainted 6.12.0 #1
> Hardware name: Raspberry Pi Compute Module 3 Rev 1.0 (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : tlb_remove_table+0xc8/0x1c0
> lr : tlb_remove_table+0xc0/0x1c0
> sp : ffff80008117b9d0
> x29: ffff80008117b9d0 x28: ffff80008117bb98 x27: 0000aaaad1df8000
> x26: 0000aaaad1df7fff x25: ffff0000044f4468 x24: 00000000ffffffdb
> x23: fffffdffc008f2c0 x22: ffff8000808a1860 x21: 0000aaaad1c00000
> x20: fffffdffc008f2c0 x19: ffff80008117bb98 x18: ffff80008117bb50
> x17: ffffffffffffffff x16: 0000000000000000 x15: ffff8000807b62a8
> x14: 0000ffffb7640fff x13: 0000000000000000 x12: 0000000000000000
> x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000037b6f
> x8 : ffff80008094a0e7 x7 : 0000000000000800 x6 : 0000000000000000
> x5 : 0000000000000801 x4 : dead000000000122 x3 : 0000000000000000
> x2 : 0000020040000000 x1 : ffff000000000000 x0 : ffbf801f8e7bf000
> Call trace:
>   tlb_remove_table+0xc8/0x1c0 (P)
>   tlb_remove_table+0xc0/0x1c0 (L)
>   free_pgd_range+0x228/0x5d0
>   free_pgtables+0x1c4/0x270
>   exit_mmap+0x130/0x2c0
>   mmput+0x70/0x190
>   do_exit+0x248/0x8f0
>   do_group_exit+0x34/0x90
>   get_signal+0x834/0x860
>   do_signal+0x118/0x330
>   do_notify_resume+0xc0/0x140
>   el0_svc+0xb8/0xd0
>   el0t_64_sync_handler+0x10c/0x140
>   el0t_64_sync+0x198/0x19c
> Code: 52850000 94006fdd f9000660 b4000080 (b900101f)
> ---[ end trace 0000000000000000 ]---
> Fixing recursive fault but reboot is needed!
> Unable to handle kernel NULL pointer dereference at virtual address
> 0000000000000008
> Mem abort info:
>    ESR = 0x0000000096000044
>    EC = 0x25: DABT (current EL), IL = 32 bits
>    SET = 0, FnV = 0
>    EA = 0, S1PTW = 0
>    FSC = 0x04: level 0 translation fault
> Data abort info:
>    ISV = 0, ISS = 0x00000044, ISS2 = 0x00000000
>    CM = 0, WnR = 1, TnD = 0, TagAccess = 0
>    GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> user pgtable: 4k pages, 48-bit VAs, pgdp=000000000238c000
> [0000000000000008] pgd=0000000000000000, p4d=0000000000000000
> Internal error: Oops: 0000000096000044 [#2] SMP
> CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Tainted: G      D            6.12.0 #1
> Tainted: [D]=DIE
> Hardware name: Raspberry Pi Compute Module 3 Rev 1.0 (DT)
> pstate: a0000005 (NzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : free_unref_page_commit+0x70/0x2c0
> lr : free_unref_page+0x17c/0x400
> sp : ffff800080003da0
> x29: ffff800080003da0 x28: 0000000000000000 x27: 0000000000000000
> x26: 0000000000000000 x25: ffff00003e39e0b8 x24: 0000000000000000
> x23: 0000000000000000 x22: 00000000ffffffff x21: ffff00003e39efc0
> x20: 0000000000000000 x19: ffff800080add340 x18: ffff800081163c40
> x17: ffff7fffbda55000 x16: ffff800080000000 x15: 0000000000001900
> x14: 00000f423fffe700 x13: 0000000000000000 x12: ffff00003e39efc0
> x11: 0000000000000001 x10: 0000000000000001 x9 : ffff7fffbda55000
> x8 : fffffdffc00e81c8 x7 : ffff00003e39efe0 x6 : 0000000000000020
> x5 : ffff00003e39efc0 x4 : 00000000ffffffff x3 : 0000000000000000
> x2 : fffffdffc00e81c0 x1 : 0000000000000000 x0 : ffff800080add340
> Call trace:
>   free_unref_page_commit+0x70/0x2c0 (P)
>   free_unref_page+0x17c/0x400 (L)
>   free_unref_page+0x17c/0x400
>   __folio_put+0x50/0xb0
>   tlb_remove_table_rcu+0xb0/0xc0
>   rcu_core+0x1f4/0x520
>   rcu_core_si+0x10/0x20
>   handle_softirqs+0xf4/0x230
>   __do_softirq+0x14/0x20
>   ____do_softirq+0x10/0x20
>   call_on_irq_stack+0x24/0x54
>   do_softirq_own_stack+0x1c/0x30
>   __irq_exit_rcu+0xc8/0x100
>   irq_exit_rcu+0x10/0x20
>   el1_interrupt+0x38/0x60
>   el1h_64_irq_handler+0x18/0x30
>   el1h_64_irq+0x6c/0x70
>   default_idle_call+0x28/0x3c (P)
>   default_idle_call+0x24/0x3c (L)
>   do_idle+0x9c/0xf0
>   cpu_startup_entry+0x34/0x40
>   rest_init+0xb8/0xc0
>   do_one_initcall+0x0/0x16c
>   __primary_switched+0x88/0x90
> Code: 8b2ac0e7 f8296827 8b0600a7 f9401181 (f9000428)
> ---[ end trace 0000000000000000 ]---
> Kernel panic - not syncing: Oops: Fatal exception in interrupt
> SMP: stopping secondary CPUs
> Kernel Offset: disabled
> CPU features: 0x000,00000c00,00800000,0200420b
> Memory Limit: none
> ---[ end Kernel panic - not syncing: Oops: Fatal exception in interrupt ]---
>
> Thanks,
> Nuno

-- 
--
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)

[-- Attachment #2: 0001-fbdev-dma-Add-shadow-buffering-for-deferred-I-O.patch --]
[-- Type: text/x-patch, Size: 9321 bytes --]

From e4e91dbee6b49c6887a5b82100e43900b24fe0a7 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Thu, 28 Nov 2024 11:11:25 +0100
Subject: [PATCH] fbdev-dma: Add shadow buffering for deferred I/O

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_fbdev_dma.c | 213 +++++++++++++++++++++++---------
 1 file changed, 153 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c
index b14b581c059d..5d4c5f5d0825 100644
--- a/drivers/gpu/drm/drm_fbdev_dma.c
+++ b/drivers/gpu/drm/drm_fbdev_dma.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: MIT
 
 #include <linux/fb.h>
+#include <linux/vmalloc.h>
 
 #include <drm/drm_drv.h>
 #include <drm/drm_fbdev_dma.h>
@@ -74,33 +75,98 @@ FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS(drm_fbdev_dma,
 				   drm_fb_helper_damage_range,
 				   drm_fb_helper_damage_area);
 
-static int drm_fbdev_dma_deferred_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
+static void drm_fbdev_dma_deferred_fb_destroy(struct fb_info *info)
 {
 	struct drm_fb_helper *fb_helper = info->par;
-	struct drm_framebuffer *fb = fb_helper->fb;
-	struct drm_gem_dma_object *dma = drm_fb_dma_get_gem_obj(fb, 0);
+	void *shadow = info->screen_buffer;
+
+	if (!fb_helper->dev)
+		return;
 
-	if (!dma->map_noncoherent)
-		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
+	if (info->fbdefio)
+		fb_deferred_io_cleanup(info);
+	drm_fb_helper_fini(fb_helper);
+	vfree(shadow);
 
-	return fb_deferred_io_mmap(info, vma);
+	drm_client_buffer_vunmap(fb_helper->buffer);
+	drm_client_framebuffer_delete(fb_helper->buffer);
+	drm_client_release(&fb_helper->client);
+	drm_fb_helper_unprepare(fb_helper);
+	kfree(fb_helper);
 }
 
 static const struct fb_ops drm_fbdev_dma_deferred_fb_ops = {
 	.owner = THIS_MODULE,
 	.fb_open = drm_fbdev_dma_fb_open,
 	.fb_release = drm_fbdev_dma_fb_release,
-	__FB_DEFAULT_DEFERRED_OPS_RDWR(drm_fbdev_dma),
+	FB_DEFAULT_DEFERRED_OPS(drm_fbdev_dma),
 	DRM_FB_HELPER_DEFAULT_OPS,
-	__FB_DEFAULT_DEFERRED_OPS_DRAW(drm_fbdev_dma),
-	.fb_mmap = drm_fbdev_dma_deferred_fb_mmap,
-	.fb_destroy = drm_fbdev_dma_fb_destroy,
+	.fb_destroy = drm_fbdev_dma_deferred_fb_destroy,
 };
 
 /*
  * struct drm_fb_helper
  */
 
+static void drm_fbdev_dma_damage_blit_real(struct drm_fb_helper *fb_helper,
+					   struct drm_clip_rect *clip,
+					   struct iosys_map *dst)
+{
+	struct drm_framebuffer *fb = fb_helper->fb;
+	size_t offset = clip->y1 * fb->pitches[0];
+	size_t len = clip->x2 - clip->x1;
+	unsigned int y;
+	void *src;
+
+	switch (drm_format_info_bpp(fb->format, 0)) {
+	case 1:
+		offset += clip->x1 / 8;
+		len = DIV_ROUND_UP(len + clip->x1 % 8, 8);
+		break;
+	case 2:
+		offset += clip->x1 / 4;
+		len = DIV_ROUND_UP(len + clip->x1 % 4, 4);
+		break;
+	case 4:
+		offset += clip->x1 / 2;
+		len = DIV_ROUND_UP(len + clip->x1 % 2, 2);
+		break;
+	default:
+		offset += clip->x1 * fb->format->cpp[0];
+		len *= fb->format->cpp[0];
+		break;
+	}
+
+	src = fb_helper->info->screen_buffer + offset;
+	iosys_map_incr(dst, offset); /* go to first pixel within clip rect */
+
+	for (y = clip->y1; y < clip->y2; y++) {
+		iosys_map_memcpy_to(dst, 0, src, len);
+		iosys_map_incr(dst, fb->pitches[0]);
+		src += fb->pitches[0];
+	}
+}
+
+static int drm_fbdev_dma_damage_blit(struct drm_fb_helper *fb_helper,
+				     struct drm_clip_rect *clip)
+{
+	struct drm_client_buffer *buffer = fb_helper->buffer;
+	struct iosys_map dst;
+
+	/*
+	 * For fbdev emulation, we only have to protect against fbdev modeset
+	 * operations. Nothing else will involve the client buffer's BO. So it
+	 * is sufficient to acquire struct drm_fb_helper.lock here.
+	 */
+	mutex_lock(&fb_helper->lock);
+
+	dst = buffer->map;
+	drm_fbdev_dma_damage_blit_real(fb_helper, clip, &dst);
+
+	mutex_unlock(&fb_helper->lock);
+
+	return 0;
+}
 static int drm_fbdev_dma_helper_fb_dirty(struct drm_fb_helper *helper,
 					 struct drm_clip_rect *clip)
 {
@@ -112,6 +178,10 @@ static int drm_fbdev_dma_helper_fb_dirty(struct drm_fb_helper *helper,
 		return 0;
 
 	if (helper->fb->funcs->dirty) {
+		ret = drm_fbdev_dma_damage_blit(helper, clip);
+		if (drm_WARN_ONCE(dev, ret, "Damage blitter failed: ret=%d\n", ret))
+			return ret;
+
 		ret = helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, clip, 1);
 		if (drm_WARN_ONCE(dev, ret, "Dirty helper failed: ret=%d\n", ret))
 			return ret;
@@ -128,14 +198,80 @@ static const struct drm_fb_helper_funcs drm_fbdev_dma_helper_funcs = {
  * struct drm_fb_helper
  */
 
+static int drm_fbdev_dma_driver_fbdev_probe_tail(struct drm_fb_helper *fb_helper,
+						 struct drm_fb_helper_surface_size *sizes)
+{
+	struct drm_device *dev = fb_helper->dev;
+	struct drm_client_buffer *buffer = fb_helper->buffer;
+	struct drm_gem_dma_object *dma_obj = to_drm_gem_dma_obj(buffer->gem);
+	struct drm_framebuffer *fb = fb_helper->fb;
+	struct fb_info *info = fb_helper->info;
+	struct iosys_map map = buffer->map;
+
+	info->fbops = &drm_fbdev_dma_fb_ops;
+
+	/* screen */
+	info->flags |= FBINFO_VIRTFB; /* system memory */
+	if (dma_obj->map_noncoherent)
+		info->flags |= FBINFO_READS_FAST; /* signal caching */
+	info->screen_size = sizes->surface_height * fb->pitches[0];
+	info->screen_buffer = map.vaddr;
+	if (!(info->flags & FBINFO_HIDE_SMEM_START)) {
+		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;
+
+	return 0;
+}
+
+static int drm_fbdev_dma_driver_fbdev_probe_tail_deferred(struct drm_fb_helper *fb_helper,
+							  struct drm_fb_helper_surface_size *sizes)
+{
+	struct drm_client_buffer *buffer = fb_helper->buffer;
+	struct fb_info *info = fb_helper->info;
+	size_t screen_size = buffer->gem->size;
+	void *screen_buffer;
+	int ret;
+
+	/*
+	 * Deferred I/O requires struct page for framebuffer memory,
+	 * which is not guaranteed for all DMA ranges. We thus create
+	 * a shadow buffer in system memory.
+	 */
+	screen_buffer = vzalloc(screen_size);
+	if (!screen_buffer)
+		return -ENOMEM;
+
+	info->fbops = &drm_fbdev_dma_deferred_fb_ops;
+
+	/* screen */
+	info->flags |= FBINFO_VIRTFB; /* system memory */
+	info->flags |= FBINFO_READS_FAST; /* signal caching */
+	info->screen_buffer = screen_buffer;
+	info->fix.smem_len = screen_size;
+
+	fb_helper->fbdefio.delay = HZ / 20;
+	fb_helper->fbdefio.deferred_io = drm_fb_helper_deferred_io;
+
+	info->fbdefio = &fb_helper->fbdefio;
+	ret = fb_deferred_io_init(info);
+	if (ret)
+		goto err_vfree;
+
+	return 0;
+
+err_vfree:
+	vfree(screen_buffer);
+	return ret;
+}
+
 int drm_fbdev_dma_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
 				     struct drm_fb_helper_surface_size *sizes)
 {
 	struct drm_client_dev *client = &fb_helper->client;
 	struct drm_device *dev = fb_helper->dev;
-	bool use_deferred_io = false;
 	struct drm_client_buffer *buffer;
-	struct drm_gem_dma_object *dma_obj;
 	struct drm_framebuffer *fb;
 	struct fb_info *info;
 	u32 format;
@@ -152,19 +288,9 @@ int drm_fbdev_dma_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
 					       sizes->surface_height, format);
 	if (IS_ERR(buffer))
 		return PTR_ERR(buffer);
-	dma_obj = to_drm_gem_dma_obj(buffer->gem);
 
 	fb = buffer->fb;
 
-	/*
-	 * Deferred I/O requires struct page for framebuffer memory,
-	 * which is not guaranteed for all DMA ranges. We thus only
-	 * install deferred I/O if we have a framebuffer that requires
-	 * it.
-	 */
-	if (fb->funcs->dirty)
-		use_deferred_io = true;
-
 	ret = drm_client_buffer_vmap(buffer, &map);
 	if (ret) {
 		goto err_drm_client_buffer_delete;
@@ -185,45 +311,12 @@ int drm_fbdev_dma_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
 
 	drm_fb_helper_fill_info(info, fb_helper, sizes);
 
-	if (use_deferred_io)
-		info->fbops = &drm_fbdev_dma_deferred_fb_ops;
+	if (fb->funcs->dirty)
+		ret = drm_fbdev_dma_driver_fbdev_probe_tail_deferred(fb_helper, sizes);
 	else
-		info->fbops = &drm_fbdev_dma_fb_ops;
-
-	/* screen */
-	info->flags |= FBINFO_VIRTFB; /* system memory */
-	if (dma_obj->map_noncoherent)
-		info->flags |= FBINFO_READS_FAST; /* signal caching */
-	info->screen_size = sizes->surface_height * fb->pitches[0];
-	info->screen_buffer = map.vaddr;
-	if (!(info->flags & FBINFO_HIDE_SMEM_START)) {
-		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;
-
-	/*
-	 * Only set up deferred I/O if the screen buffer supports
-	 * it. If this disagrees with the previous test for ->dirty,
-	 * mmap on the /dev/fb file might not work correctly.
-	 */
-	if (!is_vmalloc_addr(info->screen_buffer) && info->fix.smem_start) {
-		unsigned long pfn = info->fix.smem_start >> PAGE_SHIFT;
-
-		if (drm_WARN_ON(dev, !pfn_to_page(pfn)))
-			use_deferred_io = false;
-	}
-
-	/* deferred I/O */
-	if (use_deferred_io) {
-		fb_helper->fbdefio.delay = HZ / 20;
-		fb_helper->fbdefio.deferred_io = drm_fb_helper_deferred_io;
-
-		info->fbdefio = &fb_helper->fbdefio;
-		ret = fb_deferred_io_init(info);
-		if (ret)
-			goto err_drm_fb_helper_release_info;
-	}
+		ret = drm_fbdev_dma_driver_fbdev_probe_tail(fb_helper, sizes);
+	if (ret)
+		goto err_drm_fb_helper_release_info;
 
 	return 0;
 
-- 
2.47.0

Re: drm/fbdev-dma: regression

[Nuno Gonçalves]

On Mon, Dec 2, 2024 at 12:18 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Thanks for testing. Attached is another patch. It attempts to restore
> the fbdev handling as before the regression. Please give it a try.

This one fixes it for me. Thanks

Re: drm/fbdev-dma: regression

[Thomas Zimmermann]

Hi


Am 07.12.24 um 23:36 schrieb Nuno Gonçalves:
> On Mon, Dec 2, 2024 at 12:18 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Thanks for testing. Attached is another patch. It attempts to restore
>> the fbdev handling as before the regression. Please give it a try.
> This one fixes it for me. Thanks

Thanks you so much for testing. I'll prepare a real patch. Can I add 
your Reported-by and Tested-by tags?

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: drm/fbdev-dma: regression

[Nuno Gonçalves]

On Mon, Dec 9, 2024 at 1:43 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Thanks you so much for testing. I'll prepare a real patch. Can I add
> your Reported-by and Tested-by tags?

Reported-by: Nuno Gonçalves <nunojpg@gmail.com>
Tested-by: Nuno Gonçalves <nunojpg@gmail.com>

Thanks,
Nuno

Re: drm/fbdev-dma: regression

[Thomas Zimmermann]

Hi


Am 09.12.24 um 14:56 schrieb Nuno Gonçalves:
> On Mon, Dec 9, 2024 at 1:43 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Thanks you so much for testing. I'll prepare a real patch. Can I add
>> your Reported-by and Tested-by tags?
> Reported-by: Nuno Gonçalves <nunojpg@gmail.com>
> Tested-by: Nuno Gonçalves <nunojpg@gmail.com>

Thanks a lot. I've sent out the patch for review. Apologies if this took 
a bit longer than expected.

Best regards
Thomas

>
> Thanks,
> Nuno

-- 
--
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: drm/fbdev-dma: regression

[Nuno Gonçalves]

Dear Thomas,

Could you check if the patch got lost in review?

I can confirm that mainline is still broken since this 2024/May regression.

Thanks,
Nuno

On Wed, Dec 11, 2024 at 9:07 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
>
> Am 09.12.24 um 14:56 schrieb Nuno Gonçalves:
> > On Mon, Dec 9, 2024 at 1:43 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >> Thanks you so much for testing. I'll prepare a real patch. Can I add
> >> your Reported-by and Tested-by tags?
> > Reported-by: Nuno Gonçalves <nunojpg@gmail.com>
> > Tested-by: Nuno Gonçalves <nunojpg@gmail.com>
>
> Thanks a lot. I've sent out the patch for review. Apologies if this took
> a bit longer than expected.
>
> Best regards
> Thomas
>
> >
> > Thanks,
> > Nuno
>
> --
> --
> 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: drm/fbdev-dma: regression

[Thomas Zimmermann]

Hi

Am 26.02.25 um 23:57 schrieb Nuno Gonçalves:
> Dear Thomas,
>
> Could you check if the patch got lost in review?
>
> I can confirm that mainline is still broken since this 2024/May regression.

You're right. The patch got reviewed and then I forgot to merge it. It's 
now on its way into the upstream kernel. Thanks for the note. Apologies 
for the delay.

Best regards
Thomas

>
> Thanks,
> Nuno
>
> On Wed, Dec 11, 2024 at 9:07 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Hi
>>
>>
>> Am 09.12.24 um 14:56 schrieb Nuno Gonçalves:
>>> On Mon, Dec 9, 2024 at 1:43 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>>> Thanks you so much for testing. I'll prepare a real patch. Can I add
>>>> your Reported-by and Tested-by tags?
>>> Reported-by: Nuno Gonçalves <nunojpg@gmail.com>
>>> Tested-by: Nuno Gonçalves <nunojpg@gmail.com>
>> Thanks a lot. I've sent out the patch for review. Apologies if this took
>> a bit longer than expected.
>>
>> Best regards
>> Thomas
>>
>>> Thanks,
>>> Nuno
>> --
>> --
>> 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)