| Message ID | 20240911123507.v2.1.Id08823b2f848237ae90ce5c5fa7e027e97c33ad3@changeid |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] init: Don't proxy `console=` to earlycon | expand |
On Wed 2024-09-11 12:35:14, Raul E Rangel wrote: > Today we are proxying the `console=` command line args to the > `param_setup_earlycon()` handler. This is done because the following are > equivalent: > > console=uart[8250],mmio,<addr>[,options] > earlycon=uart[8250],mmio,<addr>[,options] > > Both invocations enable an early `bootconsole`. `console=uartXXXX` is > just an alias for `earlycon=uartXXXX`. > > In addition, when `earlycon=` (empty value) or just `earlycon` > (no value) is specified on the command line, we enable the earlycon > `bootconsole` specified by the SPCR table or the DT. > > The problem arises when `console=` (empty value) is specified on the > command line. It's intention is to disable the `console`, but what > happens instead is that the SPRC/DT console gets enabled. > > This happens because we are proxying the `console=` (empty value) > parameter to the `earlycon` handler. The `earlycon` handler then sees > that the parameter value is empty, so it enables the SPCR/DT > `bootconsole`. > > This change makes it so that the `console` or `console=` parameters no > longer enable the SPCR/DT `bootconsole`. I also cleans up the hack in > `main.c` that would forward the `console` parameter to the `earlycon` > handler. > > Signed-off-by: Raul E Rangel <rrangel@chromium.org> It like this approach. It works well: Reviewed-by: Petr Mladek <pmladek@suse.com> Tested-by: Petr Mladek <pmladek@suse.com> I could take it via the printk tree for 6.13. From my POV, it is too late for 6.12. I am sorry I have been busy with the printk rework :-( I am going to wait few days for another eventual review or taker. Best Regards, Petr
On Tue 2024-09-24 10:05:08, Raul Rangel wrote: > On Thu, Sep 19, 2024 at 6:57 AM Petr Mladek <pmladek@suse.com> wrote: > > > On Wed 2024-09-11 12:35:14, Raul E Rangel wrote: > > > Today we are proxying the `console=` command line args to the > > > `param_setup_earlycon()` handler. This is done because the following are > > > equivalent: > > > > > > console=uart[8250],mmio,<addr>[,options] > > > earlycon=uart[8250],mmio,<addr>[,options] > > > > > > Both invocations enable an early `bootconsole`. `console=uartXXXX` is > > > just an alias for `earlycon=uartXXXX`. > > > > > > In addition, when `earlycon=` (empty value) or just `earlycon` > > > (no value) is specified on the command line, we enable the earlycon > > > `bootconsole` specified by the SPCR table or the DT. > > > > > > The problem arises when `console=` (empty value) is specified on the > > > command line. It's intention is to disable the `console`, but what > > > happens instead is that the SPRC/DT console gets enabled. > > > > > > This happens because we are proxying the `console=` (empty value) > > > parameter to the `earlycon` handler. The `earlycon` handler then sees > > > that the parameter value is empty, so it enables the SPCR/DT > > > `bootconsole`. > > > > > > This change makes it so that the `console` or `console=` parameters no > > > longer enable the SPCR/DT `bootconsole`. I also cleans up the hack in > > > `main.c` that would forward the `console` parameter to the `earlycon` > > > handler. > > > > > > Signed-off-by: Raul E Rangel <rrangel@chromium.org> > > > > It like this approach. It works well: > > > > Reviewed-by: Petr Mladek <pmladek@suse.com> > > Tested-by: Petr Mladek <pmladek@suse.com> > > > > Thanks for reviewing and testing! I know it takes a significant amount of > time, so thank you. > > > > > I could take it via the printk tree for 6.13. From my POV, it is too > > late for 6.12. I am sorry I have been busy with the printk rework :-( > > > > 6.13 is fine. As long as it lands upstream I can cherry pick the patch into > our forks without any pushback. JFYI, the patch has been committed into printk/linux.git, branch for-6.13. Best Regards, Petr
On Tue, Oct 1, 2024 at 9:09 AM Petr Mladek <pmladek@suse.com> wrote: > > On Tue 2024-09-24 10:05:08, Raul Rangel wrote: > > On Thu, Sep 19, 2024 at 6:57 AM Petr Mladek <pmladek@suse.com> wrote: > > > > > On Wed 2024-09-11 12:35:14, Raul E Rangel wrote: > > > > Today we are proxying the `console=` command line args to the > > > > `param_setup_earlycon()` handler. This is done because the following are > > > > equivalent: > > > > > > > > console=uart[8250],mmio,<addr>[,options] > > > > earlycon=uart[8250],mmio,<addr>[,options] > > > > > > > > Both invocations enable an early `bootconsole`. `console=uartXXXX` is > > > > just an alias for `earlycon=uartXXXX`. > > > > > > > > In addition, when `earlycon=` (empty value) or just `earlycon` > > > > (no value) is specified on the command line, we enable the earlycon > > > > `bootconsole` specified by the SPCR table or the DT. > > > > > > > > The problem arises when `console=` (empty value) is specified on the > > > > command line. It's intention is to disable the `console`, but what > > > > happens instead is that the SPRC/DT console gets enabled. > > > > > > > > This happens because we are proxying the `console=` (empty value) > > > > parameter to the `earlycon` handler. The `earlycon` handler then sees > > > > that the parameter value is empty, so it enables the SPCR/DT > > > > `bootconsole`. > > > > > > > > This change makes it so that the `console` or `console=` parameters no > > > > longer enable the SPCR/DT `bootconsole`. I also cleans up the hack in > > > > `main.c` that would forward the `console` parameter to the `earlycon` > > > > handler. > > > > > > > > Signed-off-by: Raul E Rangel <rrangel@chromium.org> > > > > > > It like this approach. It works well: > > > > > > Reviewed-by: Petr Mladek <pmladek@suse.com> > > > Tested-by: Petr Mladek <pmladek@suse.com> > > > > > > > Thanks for reviewing and testing! I know it takes a significant amount of > > time, so thank you. > > > > > > > > I could take it via the printk tree for 6.13. From my POV, it is too > > > late for 6.12. I am sorry I have been busy with the printk rework :-( > > > > > > > 6.13 is fine. As long as it lands upstream I can cherry pick the patch into > > our forks without any pushback. > > JFYI, the patch has been committed into printk/linux.git, > branch for-6.13. Thank you! > > Best Regards, > Petr
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c index a5fbb6ed38aed6..ab9af37f6cda35 100644 --- a/drivers/tty/serial/earlycon.c +++ b/drivers/tty/serial/earlycon.c @@ -248,6 +248,29 @@ static int __init param_setup_earlycon(char *buf) } early_param("earlycon", param_setup_earlycon); +/* + * The `console` parameter is overloaded. It's handled here as an early param + * and in `printk.c` as a late param. It's possible to specify an early + * `bootconsole` using `earlycon=uartXXXX` (handled above), or via + * the `console=uartXXX` alias. See the comment in `8250_early.c`. + */ +static int __init param_setup_earlycon_console_alias(char *buf) +{ + /* + * A plain `console` parameter must not enable the SPCR `bootconsole` + * like a plain `earlycon` does. + * + * A `console=` parameter that specifies an empty value is used to + * disable the `console`, not the `earlycon` `bootconsole`. The + * disabling of the `console` is handled by `printk.c`. + */ + if (!buf || !buf[0]) + return 0; + + return param_setup_earlycon(buf); +} +early_param("console", param_setup_earlycon_console_alias); + #ifdef CONFIG_OF_EARLY_FLATTREE int __init of_setup_earlycon(const struct earlycon_id *match, diff --git a/init/main.c b/init/main.c index 206acdde51f5a9..e668dda1c68f3f 100644 --- a/init/main.c +++ b/init/main.c @@ -754,10 +754,7 @@ static int __init do_early_param(char *param, char *val, const struct obs_kernel_param *p; for (p = __setup_start; p < __setup_end; p++) { - if ((p->early && parameq(param, p->str)) || - (strcmp(param, "console") == 0 && - strcmp(p->str, "earlycon") == 0) - ) { + if (p->early && parameq(param, p->str)) { if (p->setup_func(val) != 0) pr_warn("Malformed early option '%s'\n", param); }
Today we are proxying the `console=` command line args to the `param_setup_earlycon()` handler. This is done because the following are equivalent: console=uart[8250],mmio,<addr>[,options] earlycon=uart[8250],mmio,<addr>[,options] Both invocations enable an early `bootconsole`. `console=uartXXXX` is just an alias for `earlycon=uartXXXX`. In addition, when `earlycon=` (empty value) or just `earlycon` (no value) is specified on the command line, we enable the earlycon `bootconsole` specified by the SPCR table or the DT. The problem arises when `console=` (empty value) is specified on the command line. It's intention is to disable the `console`, but what happens instead is that the SPRC/DT console gets enabled. This happens because we are proxying the `console=` (empty value) parameter to the `earlycon` handler. The `earlycon` handler then sees that the parameter value is empty, so it enables the SPCR/DT `bootconsole`. This change makes it so that the `console` or `console=` parameters no longer enable the SPCR/DT `bootconsole`. I also cleans up the hack in `main.c` that would forward the `console` parameter to the `earlycon` handler. Signed-off-by: Raul E Rangel <rrangel@chromium.org> --- I tested this patch with the following permutations: * console= * console * console=ttyS0,115200 * earlycon * earlycon=uart,mmio32,0xfedc9000 * earlycon= * console=uart,mmio32,0xfedc9000,115200n8 One unfortunate thing (unrelated to this patch) is that the `univ8250_console` registers as a real console (regardless if the legacy 0x3F8 UARTs are present) early on. This causes the `bootconsole` to get disabled and replaced with the non functional `univ8250_console`. [ 0.000000] earlycon: uart0 at MMIO32 0x00000000fedc9000 (options '') [ 0.000000] printk: legacy bootconsole [uart0] enabled ... [ 1.141835] printk: legacy console [ttyS0] enabled <-- Booo! [ 1.156500] printk: legacy bootconsole [uart0] disabled Changes in v2: - Switched to defining an early console parameter - Removed hack in main.c drivers/tty/serial/earlycon.c | 23 +++++++++++++++++++++++ init/main.c | 5 +---- 2 files changed, 24 insertions(+), 4 deletions(-)