mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: [PATCH v3 2/3] printk: Fix preferred console selection with multiple matches
Date: Thu, 06 Feb 2020 15:02:25 +1100	[thread overview]
Message-ID: <97dc50d411e10ac8aab1de0376d7a535fea8c60a.camel@kernel.crashing.org> (raw)

In the following circumstances, the rule of selecting the console
corresponding to the last "console=" entry on the command line as
the preferred console (CON_CONSDEV, ie, /dev/console) fails. This
is a specific example, but it could happen with different consoles
that have a similar name aliasing mechanism.

  - The kernel command line has both console=tty0 and console=ttyS0
    in that order (the latter with speed etc... arguments).
    This is common with some cloud setups such as Amazon Linux.

  - add_preferred_console is called early to register "uart0". In
    our case that happens from acpi_parse_spcr() on arm64 since the
    "enable_console" argument is true on that architecture. This causes
    "uart0" to become entry 0 of the console_cmdline array.

Now, because of the above, what happens is:

  - add_preferred_console is called by the cmdline parsing for tty0
    and ttyS0 respectively, thus occupying entries 1 and 2 of the
    console_cmdline array (since this happens after ACPI SPCR parsing).
    At that point preferred_console is set to 2 as expected.

  - When the tty layer kicks in, it will call register_console for tty0.
    This will match entry 1 in console_cmdline array. It isn't our preferred
    console but because it's our only console at this point, it will end up
    "first" in the consoles list.

  - When 8250 probes the actual serial port later on, it calls
    register_console for ttyS0. At that point the loop in register_console
    tries to match it with the entries in the console_cmdline array. Ideally
    this should match ttyS0 in entry 2, which is preferred, causing it to
    be inserted first and to replace tty0 as CONSDEV. However, 8250 provides
    a "match" hook in its struct console, and that hook will match "uart"
    as an alias to "ttyS". So we match uart0 at entry 0 in the array which
    is not the preferred console and will not match entry 2 which is since
    we break out of the loop on the first match. As a result, we don't set
    CONSDEV and don't insert it first, but second in the console list.

    As a result, we end up with tty0 remaining first in the array, and thus
    /dev/console going there instead of the last user specified one which
    is ttyS0.

This tentative fix register_console() to scan first for consoles
specified on the command line, and only if none is found, to then
scan for consoles specified by the architecture.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

v3. Refactor based on Petr suggestion
v2. Use a different logic to avoid calling match/setup multiple
    times as discussed with Petr.
---
 kernel/printk/console_cmdline.h |  1 +
 kernel/printk/printk.c          | 31 +++++++++++++++++++++----------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/kernel/printk/console_cmdline.h b/kernel/printk/console_cmdline.h
index 11f19c466af5..3ca74ad391d6 100644
--- a/kernel/printk/console_cmdline.h
+++ b/kernel/printk/console_cmdline.h
@@ -6,6 +6,7 @@ struct console_cmdline
 {
 	char	name[16];			/* Name of the driver	    */
 	int	index;				/* Minor dev. to use	    */
+	bool	user_specified;			/* Specified by command line vs. platform */
 	char	*options;			/* Options for the driver   */
 #ifdef CONFIG_A11Y_BRAILLE_CONSOLE
 	char	*brl_options;			/* Options for braille driver */
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 17602d7b7ffc..5cf47a7b880c 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2116,7 +2116,7 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
 #endif
 
 static int __add_preferred_console(char *name, int idx, char *options,
-				   char *brl_options)
+				   char *brl_options, bool user_specified)
 {
 	struct console_cmdline *c;
 	int i;
@@ -2131,6 +2131,8 @@ static int __add_preferred_console(char *name, int idx, char *options,
 		if (strcmp(c->name, name) == 0 && c->index == idx) {
 			if (!brl_options)
 				preferred_console = i;
+                       if (user_specified)
+                               c->user_specified = true;
 			return 0;
 		}
 	}
@@ -2140,6 +2142,7 @@ static int __add_preferred_console(char *name, int idx, char *options,
 		preferred_console = i;
 	strlcpy(c->name, name, sizeof(c->name));
 	c->options = options;
+	c->user_specified = user_specified;
 	braille_set_options(c, brl_options);
 
 	c->index = idx;
@@ -2194,7 +2197,7 @@ static int __init console_setup(char *str)
 	idx = simple_strtoul(s, NULL, 10);
 	*s = 0;
 
-	__add_preferred_console(buf, idx, options, brl_options);
+	__add_preferred_console(buf, idx, options, brl_options, true);
 	console_set_on_cmdline = 1;
 	return 1;
 }
@@ -2215,7 +2218,7 @@ __setup("console=", console_setup);
  */
 int add_preferred_console(char *name, int idx, char *options)
 {
-	return __add_preferred_console(name, idx, options, NULL);
+	return __add_preferred_console(name, idx, options, NULL, false);
 }
 
 bool console_suspend_enabled = true;
@@ -2636,7 +2639,7 @@ early_param("keep_bootcon", keep_bootcon_setup);
  * Care need to be taken with consoles that are statically
  * enabled such as netconsole
  */
-static int try_enable_new_console(struct console *newcon)
+static int try_enable_new_console(struct console *newcon, bool user_specified)
 {
 	struct console_cmdline *c;
 	int i;
@@ -2644,6 +2647,8 @@ static int try_enable_new_console(struct console *newcon)
 	for (i = 0, c = console_cmdline;
 	     i < MAX_CMDLINECONSOLES && c->name[0];
 	     i++, c++) {
+		if (c->user_specified != user_specified)
+			continue;
 		if (!newcon->match ||
 		    newcon->match(newcon, c->name, c->index, c->options) != 0) {
 			/* default matching */
@@ -2674,8 +2679,13 @@ static int try_enable_new_console(struct console *newcon)
 	/*
          * Some consoles, such as pstore and netconsole, can be enabled even
          * without matching.
+	 *
+	 * Note: We only do this test on the !user_specified pass so that such
+	 * a statically enabled console that isn't user specified gets a chance
+	 * to have its match() or setup() function called on our second pass
+	 * through this function.
          */
-        if (newcon->flags & CON_ENABLED)
+        if (newcon->flags & CON_ENABLED && !user_specified)
                 return 0;
 
 	return -ENOENT;
@@ -2752,11 +2762,12 @@ void register_console(struct console *newcon)
 		}
 	}
 
-	/*
-	 * See if this console matches one we selected on
-	 * the command line or if it was statically enabled
-	 */
-        err = try_enable_new_console(newcon);
+	/* See if this console matches one we selected on the command line */
+        err = try_enable_new_console(newcon, true);
+
+	/* If not, try to match against the platform default(s) */
+	if (err == -ENOENT)
+		err = try_enable_new_console(newcon, false);
 
         /* printk() messages are not printed to the Braille console. */
         if (err || newcon->flags & CON_BRL)



             reply	other threads:[~2020-02-06  4:02 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-06  4:02 Benjamin Herrenschmidt [this message]
2020-02-11 14:41 ` Petr Mladek
2020-02-11 15:33   ` Benjamin Herrenschmidt
2020-02-13  5:52 ` Sergey Senozhatsky
2020-02-13  8:39   ` Petr Mladek
2020-02-13  9:09     ` Sergey Senozhatsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=97dc50d411e10ac8aab1de0376d7a535fea8c60a.camel@kernel.crashing.org \
    --to=benh@kernel.crashing.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=sergey.senozhatsky.work@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®