LinuxLists.cc - [PATCH 1/2] dummycon: Add dummycon_(un)register_switch

2024-02-02 08:56:55

Subject: [PATCH 1/2] dummycon: Add dummycon_(un)register_switch_notifier

To detect switch attempts before a real console exists. This will be
used for the same purpose as dummycon_(un)register_output_notifier,
for fbcon to detect a more polite time to take over.

Signed-off-by: Daniel van Vugt <[email protected]>
---
drivers/video/console/dummycon.c | 35 +++++++++++++++++++++++++++-----
include/linux/console.h | 2 ++
2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
index 14af5d9e13..55e9b600ce 100644
--- a/drivers/video/console/dummycon.c
+++ b/drivers/video/console/dummycon.c
@@ -83,6 +83,32 @@ static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
/* Redraw, so that we get putc(s) for output done while blanked */
return 1;
}
+
+/* This is protected by the console_lock */
+static RAW_NOTIFIER_HEAD(dummycon_switch_nh);
+
+void dummycon_register_switch_notifier(struct notifier_block *nb)
+{
+ WARN_CONSOLE_UNLOCKED();
+
+ raw_notifier_chain_register(&dummycon_switch_nh, nb);
+}
+
+void dummycon_unregister_switch_notifier(struct notifier_block *nb)
+{
+ WARN_CONSOLE_UNLOCKED();
+
+ raw_notifier_chain_unregister(&dummycon_switch_nh, nb);
+}
+
+static int dummycon_switch(struct vc_data *vc)
+{
+ WARN_CONSOLE_UNLOCKED();
+
+ raw_notifier_call_chain(&dummycon_switch_nh, 0, vc);
+
+ return 0;
+}
#else
static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos) { }
static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
@@ -91,6 +117,10 @@ static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
{
return 0;
}
+static int dummycon_switch(struct vc_data *vc)
+{
+ return 0;
+}
#endif

static const char *dummycon_startup(void)
@@ -120,11 +150,6 @@ static bool dummycon_scroll(struct vc_data *vc, unsigned int top,
return false;
}

-static int dummycon_switch(struct vc_data *vc)
-{
- return 0;
-}
-
/*
* The console `switch' structure for the dummy console
*
diff --git a/include/linux/console.h b/include/linux/console.h
index 779d388af8..8fd70ae623 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -531,5 +531,7 @@ extern void console_init(void);
/* For deferred console takeover */
void dummycon_register_output_notifier(struct notifier_block *nb);
void dummycon_unregister_output_notifier(struct notifier_block *nb);
+void dummycon_register_switch_notifier(struct notifier_block *nb);
+void dummycon_unregister_switch_notifier(struct notifier_block *nb);

#endif /* _LINUX_CONSOLE_H */
--
2.43.0

2024-02-02 08:57:14

by Daniel van Vugt

[permalink] [raw]

Subject: [PATCH 2/2] fbcon: Defer console takeover for splash screens to first switch

Until now, deferred console takeover only meant defer until there is
output. But that risks stepping on the toes of userspace splash screens,
as console messages may appear before the splash screen. So check for the
"splash" parameter (as used by Plymouth) and if present then extend the
deferral until the first switch.

Closes: https://bugs.launchpad.net/bugs/1970069
Cc: Mario Limonciello <[email protected]>
Signed-off-by: Daniel van Vugt <[email protected]>
---
drivers/video/fbdev/core/fbcon.c | 32 +++++++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 63af6ab034..5b9f7635f7 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -76,6 +76,7 @@
#include <linux/crc32.h> /* For counting font checksums */
#include <linux/uaccess.h>
#include <asm/irq.h>
+#include <asm/cmdline.h>

#include "fbcon.h"
#include "fb_internal.h"
@@ -146,6 +147,7 @@ static inline void fbcon_map_override(void)

#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
static bool deferred_takeover = true;
+static int initial_console = -1;
#else
#define deferred_takeover false
#endif
@@ -3341,7 +3343,7 @@ static void fbcon_register_existing_fbs(struct work_struct *work)
console_unlock();
}

-static struct notifier_block fbcon_output_nb;
+static struct notifier_block fbcon_output_nb, fbcon_switch_nb;
static DECLARE_WORK(fbcon_deferred_takeover_work, fbcon_register_existing_fbs);

static int fbcon_output_notifier(struct notifier_block *nb,
@@ -3358,6 +3360,21 @@ static int fbcon_output_notifier(struct notifier_block *nb,

return NOTIFY_OK;
}
+
+static int fbcon_switch_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct vc_data *vc = data;
+
+ WARN_CONSOLE_UNLOCKED();
+
+ if (vc->vc_num != initial_console) {
+ dummycon_unregister_switch_notifier(&fbcon_switch_nb);
+ dummycon_register_output_notifier(&fbcon_output_nb);
+ }
+
+ return NOTIFY_OK;
+}
#endif

static void fbcon_start(void)
@@ -3370,7 +3387,14 @@ static void fbcon_start(void)

if (deferred_takeover) {
fbcon_output_nb.notifier_call = fbcon_output_notifier;
- dummycon_register_output_notifier(&fbcon_output_nb);
+ fbcon_switch_nb.notifier_call = fbcon_switch_notifier;
+ initial_console = fg_console;
+
+ if (cmdline_find_option_bool(boot_command_line, "splash"))
+ dummycon_register_switch_notifier(&fbcon_switch_nb);
+ else
+ dummycon_register_output_notifier(&fbcon_output_nb);
+
return;
}
#endif
@@ -3417,8 +3441,10 @@ void __exit fb_console_exit(void)
{
#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
console_lock();
- if (deferred_takeover)
+ if (deferred_takeover) {
dummycon_unregister_output_notifier(&fbcon_output_nb);
+ dummycon_unregister_switch_notifier(&fbcon_switch_nb);
+ }
console_unlock();

cancel_work_sync(&fbcon_deferred_takeover_work);
--
2.43.0

2024-02-02 19:46:37

by Mario Limonciello

[permalink] [raw]

Subject: Re: [PATCH 2/2] fbcon: Defer console takeover for splash screens to first switch

On 2/2/2024 02:53, Daniel van Vugt wrote:
> Until now, deferred console takeover only meant defer until there is
> output. But that risks stepping on the toes of userspace splash screens,
> as console messages may appear before the splash screen. So check for the
> "splash" parameter (as used by Plymouth) and if present then extend the
> deferral until the first switch.
>
> Closes: https://bugs.launchpad.net/bugs/1970069
> Cc: Mario Limonciello <[email protected]>
> Signed-off-by: Daniel van Vugt <[email protected]>
> ---
> drivers/video/fbdev/core/fbcon.c | 32 +++++++++++++++++++++++++++++---
> 1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 63af6ab034..5b9f7635f7 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -76,6 +76,7 @@
> #include <linux/crc32.h> /* For counting font checksums */
> #include <linux/uaccess.h>
> #include <asm/irq.h>
> +#include <asm/cmdline.h>
>
> #include "fbcon.h"
> #include "fb_internal.h"
> @@ -146,6 +147,7 @@ static inline void fbcon_map_override(void)
>
> #ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> static bool deferred_takeover = true;
> +static int initial_console = -1;
> #else
> #define deferred_takeover false
> #endif
> @@ -3341,7 +3343,7 @@ static void fbcon_register_existing_fbs(struct work_struct *work)
> console_unlock();
> }
>
> -static struct notifier_block fbcon_output_nb;
> +static struct notifier_block fbcon_output_nb, fbcon_switch_nb;
> static DECLARE_WORK(fbcon_deferred_takeover_work, fbcon_register_existing_fbs);
>
> static int fbcon_output_notifier(struct notifier_block *nb,
> @@ -3358,6 +3360,21 @@ static int fbcon_output_notifier(struct notifier_block *nb,
>
> return NOTIFY_OK;
> }
> +
> +static int fbcon_switch_notifier(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + struct vc_data *vc = data;
> +
> + WARN_CONSOLE_UNLOCKED();
> +
> + if (vc->vc_num != initial_console) {
> + dummycon_unregister_switch_notifier(&fbcon_switch_nb);
> + dummycon_register_output_notifier(&fbcon_output_nb);
> + }
> +
> + return NOTIFY_OK;
> +}
> #endif
>
> static void fbcon_start(void)
> @@ -3370,7 +3387,14 @@ static void fbcon_start(void)
>
> if (deferred_takeover) {
> fbcon_output_nb.notifier_call = fbcon_output_notifier;
> - dummycon_register_output_notifier(&fbcon_output_nb);
> + fbcon_switch_nb.notifier_call = fbcon_switch_notifier;
> + initial_console = fg_console;
> +
> + if (cmdline_find_option_bool(boot_command_line, "splash"))
> + dummycon_register_switch_notifier(&fbcon_switch_nb);

So there is a problem here that this would only apply if the distro
happened to use "splash" which some distros use something different.

I looked at the matching plymouth code [1] and they have a bunch of
variations of what they accept and what it does.

[1]
https://gitlab.freedesktop.org/plymouth/plymouth/-/blob/main/src/main.c?ref_type=heads#L888

If from the kernel side we're going to have code that caters to the
userspace behavior of plymouth we probably need to match all those cases
they do too.

Another alternative could be to make it a kernel configuration option
for which string to look for to activate this behavior.

> + else
> + dummycon_register_output_notifier(&fbcon_output_nb);
> +
> return;
> }
> #endif
> @@ -3417,8 +3441,10 @@ void __exit fb_console_exit(void)
> {
> #ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> console_lock();
> - if (deferred_takeover)
> + if (deferred_takeover) {
> dummycon_unregister_output_notifier(&fbcon_output_nb);
> + dummycon_unregister_switch_notifier(&fbcon_switch_nb);
> + }
> console_unlock();
>
> cancel_work_sync(&fbcon_deferred_takeover_work);

2024-02-06 10:12:22

by Daniel van Vugt

[permalink] [raw]