[PATCH fix for 4.19 0/3] fbcon: Fix VT switching being broken in combination with vgacon

[PATCH fix for 4.19 0/3] fbcon: Fix VT switching being broken in combination with vgacon

[Hans de Goede]

Hi Bartlomiej,

After backporting the deferred fbcon takeover patches to the 4.18
kernel for the upcoming Fedora 29 release, Fedora QA found a serious
bug caused by the fbcon takeover support.

When using classic (non EFI) boot on x86 and thus using vgacon, dummycon
is not used, so the deferred takeover never happens as that depends on
dummycon being the vt-console driver until the takeover happens.

This is fixed by the 2nd patch in this set, futher testing has shown that
that breaks the build when fb.o is a module, because of the conswitchp
symbol not being exported.  This made me realize that I never intended for
deferred fbcon takeover to be possible when fbdev+fbcon are being build as
a module (*), so instead of exporting yet more symbols, the first patch is
a preparation patch which only allows deferred fbcon takeover when builtin.

The third patch is a cleanup patch unexporting the dummycon_[un]register_output
symbols since those now no longer need to be exported.

At least patches 1 and 2 should be added to your tree for merging into 4.19,
patch 3 can go in in the 4.20 cycle if you prefer delaying it till then.

Regards,

Hans


*) which I should have realized when we hit the same issue with the
   dummycon_[un]register_output symbols
 

[PATCH fix for 4.19 1/3] fbcon: Only allow FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER if fbdev is builtin

[Hans de Goede]

Having FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER with fbdev+fbcon being build
as a module does not make much sense.

Having FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER only when fbdev+fbcon are
builtin was always the intention, hence the =y checks but they were
checking the wrong option, fbcon is build as part of fb.ko, so we must
check for FB=y.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/video/console/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index e91edef98633..787792c3d08d 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -152,7 +152,7 @@ config FRAMEBUFFER_CONSOLE_ROTATION
 
 config FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
 	bool "Framebuffer Console Deferred Takeover"
-	depends on FRAMEBUFFER_CONSOLE=y && DUMMY_CONSOLE=y
+	depends on FB=y && FRAMEBUFFER_CONSOLE && DUMMY_CONSOLE
 	help
 	  If enabled this defers the framebuffer console taking over the
 	  console from the dummy console until the first text is displayed on
-- 
2.18.0

[PATCH fix for 4.19 2/3] fbcon: Only defer console takeover if the current console driver is the dum

[Hans de Goede]

We rely on dummycon's output notifier mechanism to defer the takeover.

If say vgacon is the current console driver then dummycon will never get
used so its output notifier will also never get called and fbcon never
takes over. This commit fixes this by only deferring the console takeover
if the current console driver is the dummycon driver.

This commit also moves the entirety of fbcon_start under the console_lock,
since the conswitchp which fbcon_start now checks is protected by it.

This commit also inlines fbcon_register_output_notifier, since we now
need a #ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER in fbcon_start
anyways because of the write access to the deferred_takeover variable,
this has the added advantage that it puts the
dummycon_register_output_notifier() call directly after the "conswitchp !&dummy_con" comparison making it clear why that check is there.

Note the arch setup code will set conswitchp to either dummy_con or
vga_con, in the cases where it gets set to vga_con even though their is
no vga_con present we rely on vga_con_startup() to set conswitchp to
dummy_con. vga_con_startup() is guaranteed to happen before
fb_console_init() as it gets called as a console_initcall where as
fb_console_init() gets called as a subsys_initcall.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/video/fbdev/core/fbcon.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index e30d3a138c97..ef8b2d0b7071 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -3612,36 +3612,32 @@ static int fbcon_output_notifier(struct notifier_block *nb,
 
 	return NOTIFY_OK;
 }
-
-static void fbcon_register_output_notifier(void)
-{
-	fbcon_output_nb.notifier_call = fbcon_output_notifier;
-	dummycon_register_output_notifier(&fbcon_output_nb);
-}
-#else
-static inline void fbcon_register_output_notifier(void) {}
 #endif
 
 static void fbcon_start(void)
 {
+	WARN_CONSOLE_UNLOCKED();
+
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
+	if (conswitchp != &dummy_con)
+		deferred_takeover = false;
+
 	if (deferred_takeover) {
-		fbcon_register_output_notifier();
+		fbcon_output_nb.notifier_call = fbcon_output_notifier;
+		dummycon_register_output_notifier(&fbcon_output_nb);
 		return;
 	}
+#endif
 
 	if (num_registered_fb) {
 		int i;
 
-		console_lock();
-
 		for_each_registered_fb(i) {
 			info_idx = i;
 			break;
 		}
 
 		do_fbcon_takeover(0);
-		console_unlock();
-
 	}
 }
 
@@ -3724,8 +3720,8 @@ void __init fb_console_init(void)
 	for (i = 0; i < MAX_NR_CONSOLES; i++)
 		con2fb_map[i] = -1;
 
-	console_unlock();
 	fbcon_start();
+	console_unlock();
 }
 
 #ifdef MODULE
-- 
2.18.0

[PATCH fix for 4.19 3/3] dummycon: Stop exporting dummycon_[un]register_output_notifier

[Hans de Goede]

Now that we only allow FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER when
fbdev+fbcon are builtin exporting these is no longer necessary.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/video/console/dummycon.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
index 0254251fdd79..45ad925ad5f8 100644
--- a/drivers/video/console/dummycon.c
+++ b/drivers/video/console/dummycon.c
@@ -38,13 +38,11 @@ void dummycon_register_output_notifier(struct notifier_block *nb)
 	if (dummycon_putc_called)
 		nb->notifier_call(nb, 0, NULL);
 }
-EXPORT_SYMBOL_GPL(dummycon_register_output_notifier);
 
 void dummycon_unregister_output_notifier(struct notifier_block *nb)
 {
 	raw_notifier_chain_unregister(&dummycon_output_nh, nb);
 }
-EXPORT_SYMBOL_GPL(dummycon_unregister_output_notifier);
 
 static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos)
 {
-- 
2.18.0

Re: [PATCH fix for 4.19 0/3] fbcon: Fix VT switching being broken in combination with vgacon

[Bartlomiej Zolnierkiewicz]

On Thursday, August 02, 2018 01:28:38 PM Hans de Goede wrote:
> Hi Bartlomiej,
> 
> After backporting the deferred fbcon takeover patches to the 4.18
> kernel for the upcoming Fedora 29 release, Fedora QA found a serious
> bug caused by the fbcon takeover support.
> 
> When using classic (non EFI) boot on x86 and thus using vgacon, dummycon
> is not used, so the deferred takeover never happens as that depends on
> dummycon being the vt-console driver until the takeover happens.
> 
> This is fixed by the 2nd patch in this set, futher testing has shown that
> that breaks the build when fb.o is a module, because of the conswitchp
> symbol not being exported.  This made me realize that I never intended for
> deferred fbcon takeover to be possible when fbdev+fbcon are being build as
> a module (*), so instead of exporting yet more symbols, the first patch is
> a preparation patch which only allows deferred fbcon takeover when builtin.
> 
> The third patch is a cleanup patch unexporting the dummycon_[un]register_output
> symbols since those now no longer need to be exported.
> 
> At least patches 1 and 2 should be added to your tree for merging into 4.19,
> patch 3 can go in in the 4.20 cycle if you prefer delaying it till then.

Patches 1-3 queued for 4.19, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics