All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rajnesh Kanwal <rkanwal@rivosinc.com>
To: rkanwal@rivosinc.com
Cc: jirislaby@kernel.org, pmladek@suse.com, senozhatsky@chromium.org,
	rostedt@goodmis.org, john.ogness@linutronix.de,
	linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org
Subject: [PATCH 1/1] kernel: Match preferred console first.
Date: Mon, 13 Feb 2023 11:39:12 +0000	[thread overview]
Message-ID: <20230213113912.1237943-1-rkanwal@rivosinc.com> (raw)

For cases where the driver registers the console with the index set to -1
the `try_enable_preferred_console()` doesn't work as expected for multiple
hvc consoles.

For example with `CONFIG_HVC_RISCV_SBI` and `CONFIG_VIRTIO_CONSOLE` enabled
and cmdline set to "console=hvc0 console=hvc1" where hvc0 being RISCV_SBI
and hvc1 being VIRTIO_CONSOLE (the preferred one as per the preferred
selection logic in `__add_preferred_console`). hvc1 never gets enabled as hvc0
matches first with the driver name "hvc" and index set to -1.

Here is the hvc console structure from hvc_console.c L217.
static struct console hvc_console = {
        .name           = "hvc",
        .write          = hvc_console_print,
        .device         = hvc_console_device,
        .setup          = hvc_console_setup,
        .flags          = CON_PRINTBUFFER,
        .index          = -1,
};

The expected flow is that hvc1 should take precedence but given hvc console has
the index set to -1, the current logic will try to assign the first console that
matches, which is hvc0 and return. Never touching other possible consoles.

This change makes sure the preferred console is checked first and then iterate
the remaining list.

Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
---
 drivers/tty/hvc/hvc_console.c |  2 +-
 kernel/printk/printk.c        | 78 ++++++++++++++++++++++++-----------
 2 files changed, 56 insertions(+), 24 deletions(-)

diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index a683e21df19c..f05bb92d9419 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -230,7 +230,7 @@ static struct console hvc_console = {
  * -- index will remain -1
  * (2) we are first and the user specified no driver
  * -- index will be set to 0, then we will fail setup.
- * (3)  we are first and the user specified our driver
+ * (3) we are first and the user specified our driver
  * -- index will be set to user specified driver, and we will fail
  * (4) we are after driver, and this initcall will register us
  * -- if the user didn't specify a driver then the console will match
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index a5ed2e53547c..3be3ace255a9 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3148,6 +3148,49 @@ static int __init keep_bootcon_setup(char *str)

 early_param("keep_bootcon", keep_bootcon_setup);

+/* Tries to enable newcon console device matching it with passed in
+ * console_cmdline. In case if match fails the function return -EAGAIN
+ * meaning the caller should try with the next console_cmdline.
+ */
+static int __try_enable_console(struct console *newcon,
+				struct console_cmdline *c, bool user_specified,
+				bool preferred)
+{
+	int err;
+
+	if (c->user_specified != user_specified)
+		return -EAGAIN;
+
+	if (!newcon->match ||
+	    newcon->match(newcon, c->name, c->index, c->options) != 0) {
+		/* default matching */
+		BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
+		if (strcmp(c->name, newcon->name) != 0)
+			return -EAGAIN;
+
+		if (newcon->index >= 0 && newcon->index != c->index)
+			return -EAGAIN;
+
+		if (newcon->index < 0)
+			newcon->index = c->index;
+
+		if (_braille_register_console(newcon, c))
+			return 0;
+
+		if (newcon->setup &&
+		    (err = newcon->setup(newcon, c->options)) != 0)
+			return err;
+
+		newcon->flags |= CON_ENABLED;
+		if (preferred)
+			newcon->flags |= CON_CONSDEV;
+
+		return 0;
+	}
+
+	return -EAGAIN;
+}
+
 /*
  * This is called by register_console() to try to match
  * the newly registered console with any of the ones selected
@@ -3163,34 +3206,23 @@ static int try_enable_preferred_console(struct console *newcon,
 	struct console_cmdline *c;
 	int i, err;

+	/* Try to match preferred console first. */
+	if (preferred_console >= 0) {
+		c = &console_cmdline[preferred_console];
+		err = __try_enable_console(newcon, c, user_specified, true);
+		if (err != EAGAIN)
+			return err;
+	}
+
 	for (i = 0, c = console_cmdline;
 	     i < MAX_CMDLINECONSOLES && c->name[0];
 	     i++, c++) {
-		if (c->user_specified != user_specified)
+		if (i == preferred_console)
 			continue;
-		if (!newcon->match ||
-		    newcon->match(newcon, c->name, c->index, c->options) != 0) {
-			/* default matching */
-			BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
-			if (strcmp(c->name, newcon->name) != 0)
-				continue;
-			if (newcon->index >= 0 &&
-			    newcon->index != c->index)
-				continue;
-			if (newcon->index < 0)
-				newcon->index = c->index;
-
-			if (_braille_register_console(newcon, c))
-				return 0;

-			if (newcon->setup &&
-			    (err = newcon->setup(newcon, c->options)) != 0)
-				return err;
-		}
-		newcon->flags |= CON_ENABLED;
-		if (i == preferred_console)
-			newcon->flags |= CON_CONSDEV;
-		return 0;
+		err = __try_enable_console(newcon, c, user_specified, false);
+		if (err != EAGAIN)
+			return err;
 	}

 	/*
--
2.25.1


WARNING: multiple messages have this Message-ID (diff)
From: Rajnesh Kanwal <rkanwal@rivosinc.com>
To: rkanwal@rivosinc.com
Cc: jirislaby@kernel.org, pmladek@suse.com, senozhatsky@chromium.org,
	rostedt@goodmis.org, john.ogness@linutronix.de,
	linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org
Subject: [PATCH 1/1] kernel: Match preferred console first.
Date: Mon, 13 Feb 2023 11:39:12 +0000	[thread overview]
Message-ID: <20230213113912.1237943-1-rkanwal@rivosinc.com> (raw)

For cases where the driver registers the console with the index set to -1
the `try_enable_preferred_console()` doesn't work as expected for multiple
hvc consoles.

For example with `CONFIG_HVC_RISCV_SBI` and `CONFIG_VIRTIO_CONSOLE` enabled
and cmdline set to "console=hvc0 console=hvc1" where hvc0 being RISCV_SBI
and hvc1 being VIRTIO_CONSOLE (the preferred one as per the preferred
selection logic in `__add_preferred_console`). hvc1 never gets enabled as hvc0
matches first with the driver name "hvc" and index set to -1.

Here is the hvc console structure from hvc_console.c L217.
static struct console hvc_console = {
        .name           = "hvc",
        .write          = hvc_console_print,
        .device         = hvc_console_device,
        .setup          = hvc_console_setup,
        .flags          = CON_PRINTBUFFER,
        .index          = -1,
};

The expected flow is that hvc1 should take precedence but given hvc console has
the index set to -1, the current logic will try to assign the first console that
matches, which is hvc0 and return. Never touching other possible consoles.

This change makes sure the preferred console is checked first and then iterate
the remaining list.

Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
---
 drivers/tty/hvc/hvc_console.c |  2 +-
 kernel/printk/printk.c        | 78 ++++++++++++++++++++++++-----------
 2 files changed, 56 insertions(+), 24 deletions(-)

diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index a683e21df19c..f05bb92d9419 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -230,7 +230,7 @@ static struct console hvc_console = {
  * -- index will remain -1
  * (2) we are first and the user specified no driver
  * -- index will be set to 0, then we will fail setup.
- * (3)  we are first and the user specified our driver
+ * (3) we are first and the user specified our driver
  * -- index will be set to user specified driver, and we will fail
  * (4) we are after driver, and this initcall will register us
  * -- if the user didn't specify a driver then the console will match
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index a5ed2e53547c..3be3ace255a9 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3148,6 +3148,49 @@ static int __init keep_bootcon_setup(char *str)

 early_param("keep_bootcon", keep_bootcon_setup);

+/* Tries to enable newcon console device matching it with passed in
+ * console_cmdline. In case if match fails the function return -EAGAIN
+ * meaning the caller should try with the next console_cmdline.
+ */
+static int __try_enable_console(struct console *newcon,
+				struct console_cmdline *c, bool user_specified,
+				bool preferred)
+{
+	int err;
+
+	if (c->user_specified != user_specified)
+		return -EAGAIN;
+
+	if (!newcon->match ||
+	    newcon->match(newcon, c->name, c->index, c->options) != 0) {
+		/* default matching */
+		BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
+		if (strcmp(c->name, newcon->name) != 0)
+			return -EAGAIN;
+
+		if (newcon->index >= 0 && newcon->index != c->index)
+			return -EAGAIN;
+
+		if (newcon->index < 0)
+			newcon->index = c->index;
+
+		if (_braille_register_console(newcon, c))
+			return 0;
+
+		if (newcon->setup &&
+		    (err = newcon->setup(newcon, c->options)) != 0)
+			return err;
+
+		newcon->flags |= CON_ENABLED;
+		if (preferred)
+			newcon->flags |= CON_CONSDEV;
+
+		return 0;
+	}
+
+	return -EAGAIN;
+}
+
 /*
  * This is called by register_console() to try to match
  * the newly registered console with any of the ones selected
@@ -3163,34 +3206,23 @@ static int try_enable_preferred_console(struct console *newcon,
 	struct console_cmdline *c;
 	int i, err;

+	/* Try to match preferred console first. */
+	if (preferred_console >= 0) {
+		c = &console_cmdline[preferred_console];
+		err = __try_enable_console(newcon, c, user_specified, true);
+		if (err != EAGAIN)
+			return err;
+	}
+
 	for (i = 0, c = console_cmdline;
 	     i < MAX_CMDLINECONSOLES && c->name[0];
 	     i++, c++) {
-		if (c->user_specified != user_specified)
+		if (i == preferred_console)
 			continue;
-		if (!newcon->match ||
-		    newcon->match(newcon, c->name, c->index, c->options) != 0) {
-			/* default matching */
-			BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
-			if (strcmp(c->name, newcon->name) != 0)
-				continue;
-			if (newcon->index >= 0 &&
-			    newcon->index != c->index)
-				continue;
-			if (newcon->index < 0)
-				newcon->index = c->index;
-
-			if (_braille_register_console(newcon, c))
-				return 0;

-			if (newcon->setup &&
-			    (err = newcon->setup(newcon, c->options)) != 0)
-				return err;
-		}
-		newcon->flags |= CON_ENABLED;
-		if (i == preferred_console)
-			newcon->flags |= CON_CONSDEV;
-		return 0;
+		err = __try_enable_console(newcon, c, user_specified, false);
+		if (err != EAGAIN)
+			return err;
 	}

 	/*
--
2.25.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

             reply	other threads:[~2023-02-13 11:39 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-13 11:39 Rajnesh Kanwal [this message]
2023-02-13 11:39 ` [PATCH 1/1] kernel: Match preferred console first Rajnesh Kanwal
2023-02-14 11:42 ` Petr Mladek
2023-02-14 11:42   ` Petr Mladek

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=20230213113912.1237943-1-rkanwal@rivosinc.com \
    --to=rkanwal@rivosinc.com \
    --cc=jirislaby@kernel.org \
    --cc=john.ogness@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.