Re: [PATCH 5.10 28/63] fbcon: Disable accelerated scrolling

2021-01-07 Thread Geert Uytterhoeven
On Mon, Jan 4, 2021 at 5:58 PM Greg Kroah-Hartman
 wrote:
> From: Daniel Vetter 
>
> commit 39aead8373b3c20bb5965c024dfb51a94e526151 upstream.
>
> So ever since syzbot discovered fbcon, we have solid proof that it's
> full of bugs. And often the solution is to just delete code and remove
> features, e.g.  50145474f6ef ("fbcon: remove soft scrollback code").
>
> Now the problem is that most modern-ish drivers really only treat
> fbcon as an dumb kernel console until userspace takes over, and Oops
> printer for some emergencies. Looking at drm drivers and the basic
> vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> acceleration:
>
> - nouveau, seems to be enabled by default
> - omapdrm, when a DMM remapper exists using remapper rewriting for
>   y/xpanning
> - gma500, but that is getting deleted now for the GTT remapper trick,
>   and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
>   flag, so unused (and could be deleted already I think).
>
> No other driver supportes accelerated fbcon. And fbcon is the only

Note that there are 32 more drivers using acceleration under
drivers/video/fbdev/.

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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


[PATCH 5.10 28/63] fbcon: Disable accelerated scrolling

2021-01-04 Thread Greg Kroah-Hartman
From: Daniel Vetter 

commit 39aead8373b3c20bb5965c024dfb51a94e526151 upstream.

So ever since syzbot discovered fbcon, we have solid proof that it's
full of bugs. And often the solution is to just delete code and remove
features, e.g.  50145474f6ef ("fbcon: remove soft scrollback code").

Now the problem is that most modern-ish drivers really only treat
fbcon as an dumb kernel console until userspace takes over, and Oops
printer for some emergencies. Looking at drm drivers and the basic
vesa/efi fbdev drivers shows that only 3 drivers support any kind of
acceleration:

- nouveau, seems to be enabled by default
- omapdrm, when a DMM remapper exists using remapper rewriting for
  y/xpanning
- gma500, but that is getting deleted now for the GTT remapper trick,
  and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
  flag, so unused (and could be deleted already I think).

No other driver supportes accelerated fbcon. And fbcon is the only
user of this accel code (it's not exposed as uapi through ioctls),
which means we could garbage collect fairly enormous amounts of code
if we kill this.

Plus because syzbot only runs on virtual hardware, and none of the
drivers for that have acceleration, we'd remove a huge gap in testing.
And there's no other even remotely comprehensive testing aside from
syzbot.

This patch here just disables the acceleration code by always
redrawing when scrolling. The plan is that once this has been merged
for well over a year in released kernels, we can start to go around
and delete a lot of code.

v2:
- Drop a few more unused local variables, somehow I missed the
compiler warnings (Sam)
- Fix typo in comment (Jiri)
- add a todo entry for the cleanup (Thomas)

v3: Remove more unused variables (0day)

Reviewed-by: Tomi Valkeinen 
Reviewed-by: Thomas Zimmermann 
Reviewed-by: Greg Kroah-Hartman 
Acked-by: Sam Ravnborg 
Cc: Jiri Slaby 
Cc: Bartlomiej Zolnierkiewicz 
Cc: Greg Kroah-Hartman 
Cc: Linus Torvalds 
Cc: Ben Skeggs 
Cc: nouv...@lists.freedesktop.org
Cc: Tomi Valkeinen 
Cc: Daniel Vetter 
Cc: Jiri Slaby 
Cc: "Gustavo A. R. Silva" 
Cc: Tetsuo Handa 
Cc: Peilin Ye 
Cc: George Kennedy 
Cc: Nathan Chancellor 
Cc: Peter Rosin 
Signed-off-by: Daniel Vetter 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20201029132229.4068359-1-daniel.vet...@ffwll.ch
Signed-off-by: Greg Kroah-Hartman 

---
 Documentation/gpu/todo.rst   |   18 +++
 drivers/video/fbdev/core/fbcon.c |   45 ++-
 2 files changed, 26 insertions(+), 37 deletions(-)

--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -273,6 +273,24 @@ Contact: Daniel Vetter, Noralf Tronnes
 
 Level: Advanced
 
+Garbage collect fbdev scrolling acceleration
+
+
+Scroll acceleration is disabled in fbcon by hard-wiring p->scrollmode =
+SCROLL_REDRAW. There's a ton of code this will allow us to remove:
+- lots of code in fbcon.c
+- a bunch of the hooks in fbcon_ops, maybe the remaining hooks could be called
+  directly instead of the function table (with a switch on p->rotate)
+- fb_copyarea is unused after this, and can be deleted from all drivers
+
+Note that not all acceleration code can be deleted, since clearing and cursor
+support is still accelerated, which might be good candidates for further
+deletion projects.
+
+Contact: Daniel Vetter
+
+Level: Intermediate
+
 idr_init_base()
 ---
 
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1033,7 +1033,7 @@ static void fbcon_init(struct vc_data *v
struct vc_data *svc = *default_mode;
struct fbcon_display *t, *p = &fb_display[vc->vc_num];
int logo = 1, new_rows, new_cols, rows, cols, charcnt = 256;
-   int cap, ret;
+   int ret;
 
if (WARN_ON(info_idx == -1))
return;
@@ -1042,7 +1042,6 @@ static void fbcon_init(struct vc_data *v
con2fb_map[vc->vc_num] = info_idx;
 
info = registered_fb[con2fb_map[vc->vc_num]];
-   cap = info->flags;
 
if (logo_shown < 0 && console_loglevel <= CONSOLE_LOGLEVEL_QUIET)
logo_shown = FBCON_LOGO_DONTSHOW;
@@ -1147,11 +1146,13 @@ static void fbcon_init(struct vc_data *v
 
ops->graphics = 0;
 
-   if ((cap & FBINFO_HWACCEL_COPYAREA) &&
-   !(cap & FBINFO_HWACCEL_DISABLED))
-   p->scrollmode = SCROLL_MOVE;
-   else /* default to something safe */
-   p->scrollmode = SCROLL_REDRAW;
+   /*
+* No more hw acceleration for fbcon.
+*
+* FIXME: Garbage collect all the now dead code after sufficient time
+* has passed.
+*/
+   p->scrollmode = SCROLL_REDRAW;
 
/*
 *  ++guenther: console.c:vc_allocate() relies on initializing
@@ -1961,45 +1962,15 @@ static void updatescrollmode(struct fbco
 {
struct fbcon_ops *ops = info->fbcon_par;
int fh = vc->vc_font.height;
-   int cap = in