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
next 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: linkBe 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.