[PATCH v5 0/2] Optionally allow ttynull to be selected as a default console

[PATCH v5 0/2] Optionally allow ttynull to be selected as a default console

[adamsimonelli]

From: Adam Simonelli <adamsimonelli@gmail.com>

When switching to a CONFIG_VT=n world, at least on x86 systems,
/dev/console becomes /dev/ttyS0. This can cause some undesired effects.
/dev/console's behavior is now tied to the physical /dev/ttyS0, which when
disconnected can cause isatty() to fail when /dev/ttyS0 is disconnected,
and users who upgrade to a theoretical vt-less kernel from their
distribution who have a device such as a science instrument connected to
their /dev/ttyS0 port will suddenly see it receive kernel log messages.

When the new CONFIG_NULL_TTY_CONSOLE option is turned on, this will allow
the ttynull device to be leveraged as the default console. Distributions
that had CONFIG_VT turned on before will be able to leverage this option
to where /dev/console is still backed by a psuedo device, avoiding these
issues, without needing to enable the entire VT subsystem.

v2:
    rebase

v3:
    Clarify commit messages.

    Guard the all the register_console()s in ttynull to prevent it from being
    registered twice.

    Only change the link order if CONFIG_NULL_TTY_CONSOLE is enabled, otherwise
    use the existing order for ttynull if only CONFIG_NULL_TTY is enabled.

    Document why the link order changes in the drivers/tty/Makefile file.

    Replace #ifdefs

v4:
    Remember to actually include the changes to v3 in the cover letter.

v5:
    Correct code formatting in Makefile comment.

Adam Simonelli (2):
  ttynull: Add an option to allow ttynull to be used as a console device
  tty: Change order of ttynull to be linked sooner if enabled as a
    console.

 drivers/tty/Kconfig   | 15 ++++++++++++++-
 drivers/tty/Makefile  | 10 ++++++++++
 drivers/tty/ttynull.c | 20 +++++++++++++++++++-
 3 files changed, 43 insertions(+), 2 deletions(-)

-- 
2.45.2

[PATCH v5 1/2] ttynull: Add an option to allow ttynull to be used as a console device

[adamsimonelli]

From: Adam Simonelli <adamsimonelli@gmail.com>

The new config option, CONFIG_NULL_TTY_CONSOLE will allow ttynull to be
initialized by console_initcall() and selected as a possible console
device.

Signed-off-by: Adam Simonelli <adamsimonelli@gmail.com>
---
 drivers/tty/Kconfig   | 15 ++++++++++++++-
 drivers/tty/ttynull.c | 20 +++++++++++++++++++-
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index 63a494d36a1f..b4afae8b0e74 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -383,7 +383,20 @@ config NULL_TTY
 	  available or desired.
 
 	  In order to use this driver, you should redirect the console to this
-	  TTY, or boot the kernel with console=ttynull.
+	  TTY, boot the kernel with console=ttynull, or enable
+	  CONFIG_NULL_TTY_CONSOLE.
+
+	  If unsure, say N.
+
+config NULL_TTY_CONSOLE
+        bool "Support for console on ttynull"
+        depends on NULL_TTY=y && !VT_CONSOLE
+	help
+	  Say Y here if you want the NULL TTY to be used as a /dev/console
+	  device.
+
+	  This is similar to CONFIG_VT_CONSOLE, but without the dependency on
+	  CONFIG_VT. It uses the ttynull driver as the system console.
 
 	  If unsure, say N.
 
diff --git a/drivers/tty/ttynull.c b/drivers/tty/ttynull.c
index 6b2f7208b564..ec3dd3fd41c0 100644
--- a/drivers/tty/ttynull.c
+++ b/drivers/tty/ttynull.c
@@ -57,6 +57,13 @@ static struct tty_driver *ttynull_device(struct console *c, int *index)
 static struct console ttynull_console = {
 	.name = "ttynull",
 	.device = ttynull_device,
+
+	/*
+	 * Match the index and flags from other boot consoles when CONFIG_NULL_TTY_CONSOLE is
+	 * enabled, otherwise, use the default values for the index and flags.
+	*/
+	.index = IS_ENABLED(CONFIG_NULL_TTY_CONSOLE) ? -1 : 0,
+	.flags = IS_ENABLED(CONFIG_NULL_TTY_CONSOLE) ? CON_PRINTBUFFER : 0,
 };
 
 static int __init ttynull_init(void)
@@ -90,11 +97,22 @@ static int __init ttynull_init(void)
 	}
 
 	ttynull_driver = driver;
-	register_console(&ttynull_console);
+	if (!console_is_registered(&ttynull_console))
+		register_console(&ttynull_console);
 
 	return 0;
 }
 
+#ifdef CONFIG_NULL_TTY_CONSOLE
+static int __init ttynull_register(void)
+{
+	if (!console_is_registered(&ttynull_console))
+		register_console(&ttynull_console);
+	return 0;
+}
+console_initcall(ttynull_register);
+#endif
+
 static void __exit ttynull_exit(void)
 {
 	unregister_console(&ttynull_console);
-- 
2.45.2

Re: [PATCH v5 1/2] ttynull: Add an option to allow ttynull to be used as a console device

[Petr Mladek]

Hi Adam,

please add printk maintainers into Cc as already suggested by Andy
at https://lore.kernel.org/r/CAHp75VeBaetiQBykfLk_weBHdzZF1nWp=k8BJu+OKNp6iYRRTg@mail.gmail.com

The motivation is that the console registration code is in
kernel/printk/printk.c. It is historically pretty tricky.
Some ordering is defined rather by chance than by design.
And we should be careful when adding new rules and hacks.

On Mon 2025-02-24 07:39:14, adamsimonelli@gmail.com wrote:
> From: Adam Simonelli <adamsimonelli@gmail.com>
> 
> The new config option, CONFIG_NULL_TTY_CONSOLE will allow ttynull to be
> initialized by console_initcall() and selected as a possible console
> device.
> 
> Signed-off-by: Adam Simonelli <adamsimonelli@gmail.com>
> ---
>  drivers/tty/Kconfig   | 15 ++++++++++++++-
>  drivers/tty/ttynull.c | 20 +++++++++++++++++++-
>  2 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
> index 63a494d36a1f..b4afae8b0e74 100644
> --- a/drivers/tty/Kconfig
> +++ b/drivers/tty/Kconfig
> @@ -383,7 +383,20 @@ config NULL_TTY
>  	  available or desired.
>  
>  	  In order to use this driver, you should redirect the console to this
> -	  TTY, or boot the kernel with console=ttynull.
> +	  TTY, boot the kernel with console=ttynull, or enable
> +	  CONFIG_NULL_TTY_CONSOLE.
> +
> +	  If unsure, say N.
> +
> +config NULL_TTY_CONSOLE

It makes sense to enable this behavior by a CONFIG_ setting
but the name is misleading.

> +
> +        bool "Support for console on ttynull"
> +        depends on NULL_TTY=y && !VT_CONSOLE
> +	help
> +	  Say Y here if you want the NULL TTY to be used as a /dev/console
> +	  device.
> +
> +	  This is similar to CONFIG_VT_CONSOLE, but without the dependency on
> +	  CONFIG_VT. It uses the ttynull driver as the system console.

It is true that CONFIG_VT_CONSOLE causes that the virtual terminal
will get associated with /dev/console. But it works only "by chance".

It works because "register_console(&vt_console_driver)" in
con_init() is the first register_console() call. And it also
works only by chance because of the linking order.

Anyway, there are more similar CONFIG_ options, for example,
CONFIG_LP_CONSOLE, or CONFIG_VIRTIO_CONSOLE. And they are not
default when CONFIG_VT_CONSOLE is enabled. They are registered only
when the related console= option is defined on the command line.

I want to say that CONFIG_<BLA>_CONSOLE does not mean
that the BLA console will be registered by default.
And we should us a better descriptive name, for example,

  NULL_TTY_DEFAULT_CONSOLE
  NULL_TTY_DEV_CONSOLE

>  	  If unsure, say N.
>  
> diff --git a/drivers/tty/ttynull.c b/drivers/tty/ttynull.c
> index 6b2f7208b564..ec3dd3fd41c0 100644
> --- a/drivers/tty/ttynull.c
> +++ b/drivers/tty/ttynull.c
> @@ -57,6 +57,13 @@ static struct tty_driver *ttynull_device(struct console *c, int *index)
>  static struct console ttynull_console = {
>  	.name = "ttynull",
>  	.device = ttynull_device,
> +
> +	/*
> +	 * Match the index and flags from other boot consoles when CONFIG_NULL_TTY_CONSOLE is
> +	 * enabled, otherwise, use the default values for the index and flags.
> +	*/
> +	.index = IS_ENABLED(CONFIG_NULL_TTY_CONSOLE) ? -1 : 0,

This should not be needed. "con->index" is always initialized to "0"
for the default console, see:

static void try_enable_default_console(struct console *newcon)
{
	if (newcon->index < 0)
		newcon->index = 0;
[...]
}

> +	.flags = IS_ENABLED(CONFIG_NULL_TTY_CONSOLE) ? CON_PRINTBUFFER : 0,

This does not make much sense to me.

CON_PRINTBUFFER prevents duplicated output when the same device has
already been registered as a boot console. But ttynull does not have
a boot console variant. Also it is a "null" device. It never prints
anything. The output could never be duplicated by definition.

>  };
>  
>  static int __init ttynull_init(void)
> @@ -90,11 +97,22 @@ static int __init ttynull_init(void)
>  	}
>  
>  	ttynull_driver = driver;
> -	register_console(&ttynull_console);
> +	if (!console_is_registered(&ttynull_console))
> +		register_console(&ttynull_console);
>  
>  	return 0;
>  }
>  
> +#ifdef CONFIG_NULL_TTY_CONSOLE
> +static int __init ttynull_register(void)
> +{
> +	if (!console_is_registered(&ttynull_console))
> +		register_console(&ttynull_console);
> +	return 0;
> +}
> +console_initcall(ttynull_register);
> +#endif

This looks strange. I guess that you needed to move this into
console_initcall() because it is called earlier together
with the other console_initcall() calls for serial ports.
Otherwise, the hack with the linking order (2nd patch) did
not work.

But you needed to keep it in ttynull_init() so that ttynull
did not get registered prematurely when CONFIG_NULL_TTY_CONSOLE
was not enabled.

Sigh, it looks like a dirty hack which works rather by chance
than by design.


Thinking loudly:

The register_console() code is a historic mess. I dream about
having time to clean it up. Anyway, there are basically two
modes:

1. try_enable_default_console(newcon) is called only when
   there is no @preferred_console and there is no registered
   console with tty binding (valid con->device).

   The first register_console() caller wins. The order is defined
   by the __con_initcall section. Which is defined by the linking
   order.

   IMHO, it is quite fragile and non-intuitive.


2. try_enable_preferred_console() is called when some
   console is preferred via console_cmdline[]. The entries
   are added by __add_preferred_console() calls.

   This approach was created to handle console= command line
   parameters. But it was later used to define default consoles
   also via SPCR and device tree, see add_preferred_console()
   callers.

   It is also a bit tricky because the last added entry
   is preferred. Plus the .user_specified entries are
   preferred over the entries added via SPCR or device tree.

   Anyway, I think that the preference and ordering defined
   by console_cmdline[] array is a more intuitive approach.


My proposal is to call:

#ifdef CONFIG_NULL_TTY_DEFAULT_CONSOLE
	add_preferred_console("ttynull", 0, NULL);
#endif

somewhere in the kernel code. The question is where.
I wonder if the following would work:

#ifdef CONFIG_NULL_TTY_DEFAULT_CONSOLE
static int __init ttynull_default_console(void)
{
	add_preferred_console("ttynull", 0, NULL);
	return 0;
}
console_initcall(ttynull_register);
#endif

Best Regards,
Petr

Re: [PATCH v5 1/2] ttynull: Add an option to allow ttynull to be used as a console device

[Adam Simonelli]

On Tuesday, February 25, 2025 11:19:04 AM EST Petr Mladek wrote:
> Hi Adam,
> 
> please add printk maintainers into Cc as already suggested by Andy
> at https://lore.kernel.org/r/CAHp75VeBaetiQBykfLk_weBHdzZF1nWp=k8BJu+OKNp6iYRRTg@mail.gmail.com
> 
OK, I have added the other printk maintainers in this thread, and will update
my git send email script.
> The motivation is that the console registration code is in
> kernel/printk/printk.c. It is historically pretty tricky.
> Some ordering is defined rather by chance than by design.
> And we should be careful when adding new rules and hacks.
> 
> On Mon 2025-02-24 07:39:14, adamsimonelli@gmail.com wrote:
> > From: Adam Simonelli <adamsimonelli@gmail.com>
> > 
> > The new config option, CONFIG_NULL_TTY_CONSOLE will allow ttynull to be
> > initialized by console_initcall() and selected as a possible console
> > device.
> > 
> > Signed-off-by: Adam Simonelli <adamsimonelli@gmail.com>
> > ---
> >  drivers/tty/Kconfig   | 15 ++++++++++++++-
> >  drivers/tty/ttynull.c | 20 +++++++++++++++++++-
> >  2 files changed, 33 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
> > index 63a494d36a1f..b4afae8b0e74 100644
> > --- a/drivers/tty/Kconfig
> > +++ b/drivers/tty/Kconfig
> > @@ -383,7 +383,20 @@ config NULL_TTY
> >  	  available or desired.
> >  
> >  	  In order to use this driver, you should redirect the console to this
> > -	  TTY, or boot the kernel with console=ttynull.
> > +	  TTY, boot the kernel with console=ttynull, or enable
> > +	  CONFIG_NULL_TTY_CONSOLE.
> > +
> > +	  If unsure, say N.
> > +
> > +config NULL_TTY_CONSOLE
> 
> It makes sense to enable this behavior by a CONFIG_ setting
> but the name is misleading.
> 
> > +
> > +        bool "Support for console on ttynull"
> > +        depends on NULL_TTY=y && !VT_CONSOLE
> > +	help
> > +	  Say Y here if you want the NULL TTY to be used as a /dev/console
> > +	  device.
> > +
> > +	  This is similar to CONFIG_VT_CONSOLE, but without the dependency on
> > +	  CONFIG_VT. It uses the ttynull driver as the system console.
> 
> It is true that CONFIG_VT_CONSOLE causes that the virtual terminal
> will get associated with /dev/console. But it works only "by chance".
> 
> It works because "register_console(&vt_console_driver)" in
> con_init() is the first register_console() call. And it also
> works only by chance because of the linking order.
> 
> Anyway, there are more similar CONFIG_ options, for example,
> CONFIG_LP_CONSOLE, or CONFIG_VIRTIO_CONSOLE. And they are not
> default when CONFIG_VT_CONSOLE is enabled. They are registered only
> when the related console= option is defined on the command line.
> 
> I want to say that CONFIG_<BLA>_CONSOLE does not mean
> that the BLA console will be registered by default.
> And we should us a better descriptive name, for example,
> 
>   NULL_TTY_DEFAULT_CONSOLE
>   NULL_TTY_DEV_CONSOLE
> 
OK. I can change the option name.
> >  	  If unsure, say N.
> >  
> > diff --git a/drivers/tty/ttynull.c b/drivers/tty/ttynull.c
> > index 6b2f7208b564..ec3dd3fd41c0 100644
> > --- a/drivers/tty/ttynull.c
> > +++ b/drivers/tty/ttynull.c
> > @@ -57,6 +57,13 @@ static struct tty_driver *ttynull_device(struct console *c, int *index)
> >  static struct console ttynull_console = {
> >  	.name = "ttynull",
> >  	.device = ttynull_device,
> > +
> > +	/*
> > +	 * Match the index and flags from other boot consoles when CONFIG_NULL_TTY_CONSOLE is
> > +	 * enabled, otherwise, use the default values for the index and flags.
> > +	*/
> > +	.index = IS_ENABLED(CONFIG_NULL_TTY_CONSOLE) ? -1 : 0,
> 
> This should not be needed. "con->index" is always initialized to "0"
> for the default console, see:
> 
OK, I had this in an #ifdef before, it was the cleanest way to set it to -1
that I could think of, other than the ifdef... If I still need this, I will try
to think of something else to set it to -1 when the option is enabled
> static void try_enable_default_console(struct console *newcon)
> {
> 	if (newcon->index < 0)
> 		newcon->index = 0;
> [...]
> }
> 
> > +	.flags = IS_ENABLED(CONFIG_NULL_TTY_CONSOLE) ? CON_PRINTBUFFER : 0,
> 
> This does not make much sense to me.
> 
> CON_PRINTBUFFER prevents duplicated output when the same device has
> already been registered as a boot console. But ttynull does not have
> a boot console variant. Also it is a "null" device. It never prints
> anything. The output could never be duplicated by definition.
> 
OK, I was duplicating what I saw in other consoles. I can try to remove it
> >  };
> >  
> >  static int __init ttynull_init(void)
> > @@ -90,11 +97,22 @@ static int __init ttynull_init(void)
> >  	}
> >  
> >  	ttynull_driver = driver;
> > -	register_console(&ttynull_console);
> > +	if (!console_is_registered(&ttynull_console))
> > +		register_console(&ttynull_console);
> >  
> >  	return 0;
> >  }
> >  
> > +#ifdef CONFIG_NULL_TTY_CONSOLE
> > +static int __init ttynull_register(void)
> > +{
> > +	if (!console_is_registered(&ttynull_console))
> > +		register_console(&ttynull_console);
> > +	return 0;
> > +}
> > +console_initcall(ttynull_register);
> > +#endif
> 
> This looks strange. I guess that you needed to move this into
> console_initcall() because it is called earlier together
> with the other console_initcall() calls for serial ports.
> Otherwise, the hack with the linking order (2nd patch) did
> not work.
> 
> But you needed to keep it in ttynull_init() so that ttynull
> did not get registered prematurely when CONFIG_NULL_TTY_CONSOLE
> was not enabled.
> 
> Sigh, it looks like a dirty hack which works rather by chance
> than by design.
> 
> 
> Thinking loudly:
> 
> The register_console() code is a historic mess. I dream about
> having time to clean it up. Anyway, there are basically two
> modes:
> 
> 1. try_enable_default_console(newcon) is called only when
>    there is no @preferred_console and there is no registered
>    console with tty binding (valid con->device).
> 
>    The first register_console() caller wins. The order is defined
>    by the __con_initcall section. Which is defined by the linking
>    order.
> 
>    IMHO, it is quite fragile and non-intuitive.
> 
> 
> 2. try_enable_preferred_console() is called when some
>    console is preferred via console_cmdline[]. The entries
>    are added by __add_preferred_console() calls.
> 
>    This approach was created to handle console= command line
>    parameters. But it was later used to define default consoles
>    also via SPCR and device tree, see add_preferred_console()
>    callers.
> 
>    It is also a bit tricky because the last added entry
>    is preferred. Plus the .user_specified entries are
>    preferred over the entries added via SPCR or device tree.
> 
>    Anyway, I think that the preference and ordering defined
>    by console_cmdline[] array is a more intuitive approach.
> 
> 
> My proposal is to call:
> 
> #ifdef CONFIG_NULL_TTY_DEFAULT_CONSOLE
> 	add_preferred_console("ttynull", 0, NULL);
> #endif
> 
> somewhere in the kernel code. The question is where.
> I wonder if the following would work:
> 

> #ifdef CONFIG_NULL_TTY_DEFAULT_CONSOLE
> static int __init ttynull_default_console(void)
> {
> 	add_preferred_console("ttynull", 0, NULL);
> 	return 0;
> }
> console_initcall(ttynull_register);
> #endif
> 
> Best Regards,
> Petr
> 
OK, actually in earlier revisions locally, I did actually have



diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index dddb15f48d59..c1554a789de8 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3712,6 +3712,11 @@ void __init console_init(void)
 	initcall_t call;
 	initcall_entry_t *ce;
 
+#ifdef CONFIG_NULL_TTY_CONSOLE
+       if (!strstr(boot_command_line, "console="))
+               add_preferred_console("ttynull", 0, NULL);
+#endif
+
 	/* Setup the default TTY line discipline. */
 	n_tty_init();
 


Which worked as far as I could tell, at least on x86. Not sure if that was the
right place, and yeah, I was trying to better copy how CONFIG_VT_CONSOLE worked
because I thought that was more correct.

Re: [PATCH v5 1/2] ttynull: Add an option to allow ttynull to be used as a console device

[Andy Shevchenko]

On Wed, Feb 26, 2025 at 3:39 PM Adam Simonelli <adamsimonelli@gmail.com> wrote:
> On Tuesday, February 25, 2025 11:19:04 AM EST Petr Mladek wrote:

...

> > My proposal is to call:
> >
> > #ifdef CONFIG_NULL_TTY_DEFAULT_CONSOLE
> >       add_preferred_console("ttynull", 0, NULL);
> > #endif
> >
> > somewhere in the kernel code. The question is where.
> > I wonder if the following would work:
> >
>
> > #ifdef CONFIG_NULL_TTY_DEFAULT_CONSOLE
> > static int __init ttynull_default_console(void)
> > {
> >       add_preferred_console("ttynull", 0, NULL);
> >       return 0;
> > }
> > console_initcall(ttynull_register);
> > #endif
> >
> OK, actually in earlier revisions locally, I did actually have
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index dddb15f48d59..c1554a789de8 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -3712,6 +3712,11 @@ void __init console_init(void)
>         initcall_t call;
>         initcall_entry_t *ce;
>
> +#ifdef CONFIG_NULL_TTY_CONSOLE
> +       if (!strstr(boot_command_line, "console="))

Just a side note: strstr() is fragile as theoretically "console=" can
be part of an argument unrelated to the console, like
foo="bar,baz,console=10,key=value". Although I haven't checked if this
is allowed by cmdline parser (lib/cmdline.c).

> +               add_preferred_console("ttynull", 0, NULL);
> +#endif
> +
>         /* Setup the default TTY line discipline. */
>         n_tty_init();
>
>
>
> Which worked as far as I could tell, at least on x86. Not sure if that was the
> right place, and yeah, I was trying to better copy how CONFIG_VT_CONSOLE worked
> because I thought that was more correct.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 1/2] ttynull: Add an option to allow ttynull to be used as a console device

[Adam Simonelli]

On Wednesday, February 26, 2025 2:22:20 PM EST Andy Shevchenko wrote:
> On Wed, Feb 26, 2025 at 3:39 PM Adam Simonelli <adamsimonelli@gmail.com> wrote:
> > On Tuesday, February 25, 2025 11:19:04 AM EST Petr Mladek wrote:
> 
> ...
> 
> > > My proposal is to call:
> > >
> > > #ifdef CONFIG_NULL_TTY_DEFAULT_CONSOLE
> > >       add_preferred_console("ttynull", 0, NULL);
> > > #endif
> > >
> > > somewhere in the kernel code. The question is where.
> > > I wonder if the following would work:
> > >
> >
> > > #ifdef CONFIG_NULL_TTY_DEFAULT_CONSOLE
> > > static int __init ttynull_default_console(void)
> > > {
> > >       add_preferred_console("ttynull", 0, NULL);
> > >       return 0;
> > > }
> > > console_initcall(ttynull_register);
> > > #endif
> > >
> > OK, actually in earlier revisions locally, I did actually have
> >
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index dddb15f48d59..c1554a789de8 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -3712,6 +3712,11 @@ void __init console_init(void)
> >         initcall_t call;
> >         initcall_entry_t *ce;
> >
> > +#ifdef CONFIG_NULL_TTY_CONSOLE
> > +       if (!strstr(boot_command_line, "console="))
> 
> Just a side note: strstr() is fragile as theoretically "console=" can
> be part of an argument unrelated to the console, like
> foo="bar,baz,console=10,key=value". Although I haven't checked if this
> is allowed by cmdline parser (lib/cmdline.c).
> 
Dang, good call. As a crude test, console=ttynull= results in a panic, so it
does look like it allows ='s in parameter values, as it looks like it is
handling the =...

Gotta find a better way to parse it if I'm to do the `add_preferred_console`
route, Maybe I can try get_option...

What do you think of the placement of it too?
> > +               add_preferred_console("ttynull", 0, NULL);
> > +#endif
> > +
> >         /* Setup the default TTY line discipline. */
> >         n_tty_init();
> >
> >
> >
> > Which worked as far as I could tell, at least on x86. Not sure if that was the
> > right place, and yeah, I was trying to better copy how CONFIG_VT_CONSOLE worked
> > because I thought that was more correct.
> 
> 

Re: [PATCH v5 1/2] ttynull: Add an option to allow ttynull to be used as a console device

[Andy Shevchenko]

On Fri, Feb 28, 2025 at 6:39 AM Adam Simonelli <adamsimonelli@gmail.com> wrote:
>
> On Wednesday, February 26, 2025 2:22:20 PM EST Andy Shevchenko wrote:
> > On Wed, Feb 26, 2025 at 3:39 PM Adam Simonelli <adamsimonelli@gmail.com> wrote:

...

> > Just a side note: strstr() is fragile as theoretically "console=" can
> > be part of an argument unrelated to the console, like
> > foo="bar,baz,console=10,key=value". Although I haven't checked if this
> > is allowed by cmdline parser (lib/cmdline.c).
> >
> Dang, good call. As a crude test, console=ttynull= results in a panic, so it
> does look like it allows ='s in parameter values, as it looks like it is
> handling the =...
>
> Gotta find a better way to parse it if I'm to do the `add_preferred_console`
> route, Maybe I can try get_option...

It's rather next_arg() and its derivatives. I dunno if we have a
helper for the "give me an array of this options from the command
line", but it might be somewhere lurking as a static function. If that
 is the case, one may make it global and visible to other C-files
inside the kernel.

> What do you think of the placement of it too?
No particular ideas.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 1/2] ttynull: Add an option to allow ttynull to be used as a console device

[Petr Mladek]

On Wed 2025-02-26 08:39:23, Adam Simonelli wrote:
> On Tuesday, February 25, 2025 11:19:04 AM EST Petr Mladek wrote:
> > On Mon 2025-02-24 07:39:14, adamsimonelli@gmail.com wrote:
> > > From: Adam Simonelli <adamsimonelli@gmail.com>
> > > 
> > > The new config option, CONFIG_NULL_TTY_CONSOLE will allow ttynull to be
> > > initialized by console_initcall() and selected as a possible console
> > > device.
> > > 
> > > diff --git a/drivers/tty/ttynull.c b/drivers/tty/ttynull.c
> > > index 6b2f7208b564..ec3dd3fd41c0 100644
> > > --- a/drivers/tty/ttynull.c
> > > +++ b/drivers/tty/ttynull.c
> > > @@ -57,6 +57,13 @@ static struct tty_driver *ttynull_device(struct console *c, int *index)
> > >  static struct console ttynull_console = {
> > >  	.name = "ttynull",
> > >  	.device = ttynull_device,
> > > +
> > > +	/*
> > > +	 * Match the index and flags from other boot consoles when CONFIG_NULL_TTY_CONSOLE is
> > > +	 * enabled, otherwise, use the default values for the index and flags.
> > > +	*/
> > > +	.index = IS_ENABLED(CONFIG_NULL_TTY_CONSOLE) ? -1 : 0,
> > 
> > This should not be needed. "con->index" is always initialized to "0"
> > for the default console, see:
> > 
> OK, I had this in an #ifdef before, it was the cleanest way to set it to -1
> that I could think of, other than the ifdef... If I still need this, I will try
> to think of something else to set it to -1 when the option is enabled

Ah, I was not clear enough. It should be perfectly fine to always
statically initialize the value to -1. We should not need any
#ifdef or IS_ENABLED.

I mean to do:

static struct console ttynull_console = {
	.name = "ttynull",
	.device = ttynull_device,
	.index = -1,
};

We might even do this in a separate patch. IMHO, it should have
been done this way since the beginning.

> > static void try_enable_default_console(struct console *newcon)
> > {
> > 	if (newcon->index < 0)
> > 		newcon->index = 0;
> > [...]
> > }
> > 
> > > +	.flags = IS_ENABLED(CONFIG_NULL_TTY_CONSOLE) ? CON_PRINTBUFFER : 0,
> > 
> > This does not make much sense to me.
> > 
> > CON_PRINTBUFFER prevents duplicated output when the same device has
> > already been registered as a boot console. But ttynull does not have
> > a boot console variant. Also it is a "null" device. It never prints
> > anything. The output could never be duplicated by definition.
> > 
> OK, I was duplicating what I saw in other consoles. I can try to remove it

Again, I was not clear enough. My primary concern was that it did not make
much sense to use the IS_ENABLED() check and initialize the value
different way.

Anyway, I would omit the flag. It is a NULL device. It does not matter
whether it prints existing (old) messages during registration or not.

> > >  };
> > >  
> > My proposal is to call:
> > 
> > #ifdef CONFIG_NULL_TTY_DEFAULT_CONSOLE
> > static int __init ttynull_default_console(void)
> > {
> > 	add_preferred_console("ttynull", 0, NULL);
> > 	return 0;
> > }
> > console_initcall(ttynull_register);
> > #endif
> > 
> OK, actually in earlier revisions locally, I did actually have
> 
> 
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index dddb15f48d59..c1554a789de8 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -3712,6 +3712,11 @@ void __init console_init(void)
>  	initcall_t call;
>  	initcall_entry_t *ce;
>  
> +#ifdef CONFIG_NULL_TTY_CONSOLE
> +       if (!strstr(boot_command_line, "console="))
> +               add_preferred_console("ttynull", 0, NULL);

Good point! We should call add_preferred_console() only when
the is no console= command line parameter. Otherwise, it could
not get overridden by the command line.

We could check "console_set_on_cmdline", similar to
xenfb_make_preferred_console().

> +#endif
> +
>  	/* Setup the default TTY line discipline. */
>  	n_tty_init();
> 
> Which worked as far as I could tell, at least on x86. Not sure if that was the
> right place,

I would prefer to keep it in drivers/tty/ttynull.c when possible.
The following might do the trick:

#ifdef CONFIG_NULL_TTY_DEFAULT_CONSOLE
static int __init ttynull_default_console(void)
{
	if (!console_set_on_cmdline)
		add_preferred_console("ttynull", 0, NULL);

	return 0;
}
console_initcall(ttynull_register);
#endif

Best Regards,
Petr

Re: [PATCH v5 1/2] ttynull: Add an option to allow ttynull to be used as a console device

[Adam Simonelli]

On Monday, March 3, 2025 8:20:11 AM EST Petr Mladek wrote:
> On Wed 2025-02-26 08:39:23, Adam Simonelli wrote:
> > On Tuesday, February 25, 2025 11:19:04 AM EST Petr Mladek wrote:
> > > On Mon 2025-02-24 07:39:14, adamsimonelli@gmail.com wrote:
> > > > From: Adam Simonelli <adamsimonelli@gmail.com>
> > > > 
> > > > The new config option, CONFIG_NULL_TTY_CONSOLE will allow ttynull to be
> > > > initialized by console_initcall() and selected as a possible console
> > > > device.
> > > > 
> > > > diff --git a/drivers/tty/ttynull.c b/drivers/tty/ttynull.c
> > > > index 6b2f7208b564..ec3dd3fd41c0 100644
> > > > --- a/drivers/tty/ttynull.c
> > > > +++ b/drivers/tty/ttynull.c
> > > > @@ -57,6 +57,13 @@ static struct tty_driver *ttynull_device(struct console *c, int *index)
> > > >  static struct console ttynull_console = {
> > > >  	.name = "ttynull",
> > > >  	.device = ttynull_device,
> > > > +
> > > > +	/*
> > > > +	 * Match the index and flags from other boot consoles when CONFIG_NULL_TTY_CONSOLE is
> > > > +	 * enabled, otherwise, use the default values for the index and flags.
> > > > +	*/
> > > > +	.index = IS_ENABLED(CONFIG_NULL_TTY_CONSOLE) ? -1 : 0,
> > > 
> > > This should not be needed. "con->index" is always initialized to "0"
> > > for the default console, see:
> > > 
> > OK, I had this in an #ifdef before, it was the cleanest way to set it to -1
> > that I could think of, other than the ifdef... If I still need this, I will try
> > to think of something else to set it to -1 when the option is enabled
> 
> Ah, I was not clear enough. It should be perfectly fine to always
> statically initialize the value to -1. We should not need any
> #ifdef or IS_ENABLED.
> 
> I mean to do:
> 
> static struct console ttynull_console = {
> 	.name = "ttynull",
> 	.device = ttynull_device,
> 	.index = -1,
> };
> 
> We might even do this in a separate patch. IMHO, it should have
> been done this way since the beginning.
> 
OK, will do. This makes sense.
> > > static void try_enable_default_console(struct console *newcon)
> > > {
> > > 	if (newcon->index < 0)
> > > 		newcon->index = 0;
> > > [...]
> > > }
> > > 
> > > > +	.flags = IS_ENABLED(CONFIG_NULL_TTY_CONSOLE) ? CON_PRINTBUFFER : 0,
> > > 
> > > This does not make much sense to me.
> > > 
> > > CON_PRINTBUFFER prevents duplicated output when the same device has
> > > already been registered as a boot console. But ttynull does not have
> > > a boot console variant. Also it is a "null" device. It never prints
> > > anything. The output could never be duplicated by definition.
> > > 
> > OK, I was duplicating what I saw in other consoles. I can try to remove it
> 
> Again, I was not clear enough. My primary concern was that it did not make
> much sense to use the IS_ENABLED() check and initialize the value
> different way.
> 
> Anyway, I would omit the flag. It is a NULL device. It does not matter
> whether it prints existing (old) messages during registration or not.
> 
Understood, this makes sense.
> > > >  };
> > > >  
> > > My proposal is to call:
> > > 
> > > #ifdef CONFIG_NULL_TTY_DEFAULT_CONSOLE
> > > static int __init ttynull_default_console(void)
> > > {
> > > 	add_preferred_console("ttynull", 0, NULL);
> > > 	return 0;
> > > }
> > > console_initcall(ttynull_register);
> > > #endif
> > > 
> > OK, actually in earlier revisions locally, I did actually have
> > 
> > 
> > 
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index dddb15f48d59..c1554a789de8 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -3712,6 +3712,11 @@ void __init console_init(void)
> >  	initcall_t call;
> >  	initcall_entry_t *ce;
> >  
> > +#ifdef CONFIG_NULL_TTY_CONSOLE
> > +       if (!strstr(boot_command_line, "console="))
> > +               add_preferred_console("ttynull", 0, NULL);
> 
> Good point! We should call add_preferred_console() only when
> the is no console= command line parameter. Otherwise, it could
> not get overridden by the command line.
> 
> We could check "console_set_on_cmdline", similar to
> xenfb_make_preferred_console().
> 
> > +#endif
> > +
> >  	/* Setup the default TTY line discipline. */
> >  	n_tty_init();
> > 
> > Which worked as far as I could tell, at least on x86. Not sure if that was the
> > right place,
> 
> I would prefer to keep it in drivers/tty/ttynull.c when possible.
> The following might do the trick:
> 
> #ifdef CONFIG_NULL_TTY_DEFAULT_CONSOLE
> static int __init ttynull_default_console(void)
> {
> 	if (!console_set_on_cmdline)
> 		add_preferred_console("ttynull", 0, NULL);
> 
> 	return 0;
> }
> console_initcall(ttynull_register);
> #endif
> 
> Best Regards,
> Petr
> 
Thanks for that, that works.

[PATCH v5 2/2] tty: Change order of ttynull to be linked sooner if enabled as a console.

[adamsimonelli]

From: Adam Simonelli <adamsimonelli@gmail.com>

If CONFIG_NULL_TTY_CONSOLE is enabled, and CONFIG_VT is disabled, ttynull
will become the default primary console device, based on the link order.

Many distributions ship with CONFIG_VT enabled. On tested desktop hardware
if CONFIG_VT is disabled, the default console device falls back to
/dev/ttyS0 instead of /dev/tty.

This could cause issues in user space, and hardware problems:

1. The user space issues include the case where  /dev/ttyS0 is
disconnected, and the TCGETS ioctl, which some user space libraries use
as a probe to determine if a file is a tty, is called on /dev/console and
fails. Programs that call isatty() on /dev/console and get an incorrect
false value may skip expected logging to /dev/console

2. The hardware issues include the case if a user has a science instrument
or other device connected to the /dev/ttyS0 port, and they were to upgrade
to a kernel that is disabling the CONFIG_VT option, kernel logs will then be
sent to the device connected to /dev/ttyS0 unless they edit their kernel
command line manually.

The new CONFIG_NULL_TTY_CONSOLE option will give users and distribution
maintainers an option to avoid this. Disabling CONFIG_VT and enabling
CONFIG_NULL_TTY_CONSOLE will ensure the default kernel console behavior
is not dependant on hardware configuration by default, and avoid
unexpected new behavior on devices connected to the /dev/ttyS0 serial
port.

Signed-off-by: Adam Simonelli <adamsimonelli@gmail.com>
---
 drivers/tty/Makefile | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
index 07aca5184a55..1fb905cfb420 100644
--- a/drivers/tty/Makefile
+++ b/drivers/tty/Makefile
@@ -11,6 +11,10 @@ obj-$(CONFIG_N_HDLC)		+= n_hdlc.o
 obj-$(CONFIG_N_GSM)		+= n_gsm.o
 
 obj-y				+= vt/
+# If ttynull is configured to be a console by default, ensure that it is linked
+# earlier before a real one is selected.
+obj-$(CONFIG_NULL_TTY_CONSOLE)	+= ttynull.o
+
 obj-$(CONFIG_HVC_DRIVER)	+= hvc/
 obj-y				+= serial/
 obj-$(CONFIG_SERIAL_DEV_BUS)	+= serdev/
@@ -20,7 +24,13 @@ obj-$(CONFIG_AMIGA_BUILTIN_SERIAL) += amiserial.o
 obj-$(CONFIG_MOXA_INTELLIO)	+= moxa.o
 obj-$(CONFIG_MOXA_SMARTIO)	+= mxser.o
 obj-$(CONFIG_NOZOMI)		+= nozomi.o
+
+# If ttynull is enabled, but not as a boot console, it is linked and used later
+# after the real ones.
+ifneq ($(CONFIG_NULL_TTY_CONSOLE),y)
 obj-$(CONFIG_NULL_TTY)	        += ttynull.o
+endif
+
 obj-$(CONFIG_SYNCLINK_GT)	+= synclink_gt.o
 obj-$(CONFIG_PPC_EPAPR_HV_BYTECHAN) += ehv_bytechan.o
 obj-$(CONFIG_GOLDFISH_TTY)	+= goldfish.o
-- 
2.45.2