| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAE9FiQXhbR-g0SPeZ=CPGXuRGk0KrdpvaPXK_JDgg58OGk0EbA@mail.gmail.com>
Date: Sun, 5 Apr 2015 00:09:44 -0700
From: Yinghai Lu <yinghai@...nel.org>
To: Peter Hurley <peter@...leysoftware.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Jiri Slaby <jslaby@...e.cz>
Subject: Re: [PATCH v4] earlycon: 8250: Fix command line regression
On Sat, Apr 4, 2015 at 10:19 AM, Peter Hurley <peter@...leysoftware.com> wrote:
...
> Documentation/kernel-parameters.txt | 18 +++++++++++++++---
> drivers/tty/serial/8250/8250_core.c | 37 +++++++++++++++++++++++++++++++++---
> drivers/tty/serial/8250/8250_early.c | 19 ------------------
> 3 files changed, 49 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index bfcb1a6..1facf0b 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -713,10 +713,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>
> uart[8250],io,<addr>[,options]
> uart[8250],mmio,<addr>[,options]
> + uart[8250],mmio32,<addr>[,options]
> + uart[8250],0x<addr>[,options]
put this to other patch please.
> Start an early, polled-mode console on the 8250/16550
> UART at the specified I/O port or MMIO address,
> - switching to the matching ttyS device later. The
> - options are the same as for ttyS, above.
> + switching to the matching ttyS device later.
> + MMIO inter-register address stride is either 8-bit
> + (mmio) or 32-bit (mmio32).
> + If none of [io|mmio|mmio32], <addr> is assumed to be
> + equivalent to 'mmio'. 'options' are specified in the
> + same format described for ttyS above; if unspecified,
> + the h/w is not re-initialized.
> +
> hvc<n> Use the hypervisor console device <n>. This is for
> both Xen and PowerPC hypervisors.
>
> @@ -944,11 +952,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> uart[8250],io,<addr>[,options]
> uart[8250],mmio,<addr>[,options]
> uart[8250],mmio32,<addr>[,options]
> + uart[8250],0x<addr>[,options]
and this to another patch please.
> Start an early, polled-mode console on the 8250/16550
> UART at the specified I/O port or MMIO address.
> MMIO inter-register address stride is either 8-bit
> (mmio) or 32-bit (mmio32).
> - The options are the same as for ttyS, above.
> + If none of [io|mmio|mmio32], <addr> is assumed to be
> + equivalent to 'mmio'. 'options' are specified in the
> + same format described for "console=ttyS<n>"; if
> + unspecified, the h/w is not initialized.
>
> pl011,<addr>
> Start an early, polled-mode console on a pl011 serial
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index e0fb5f0..456606f 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -3447,6 +3447,21 @@ static int univ8250_console_setup(struct console *co, char *options)
> return serial8250_console_setup(up, options);
> }
>
> +static unsigned int probe_baud(struct uart_port *port)
> +{
> + unsigned char lcr, dll, dlm;
> + unsigned int quot;
> +
> + lcr = serial_port_in(port, UART_LCR);
> + serial_port_out(port, UART_LCR, lcr | UART_LCR_DLAB);
> + dll = serial_port_in(port, UART_DLL);
> + dlm = serial_port_in(port, UART_DLM);
> + serial_port_out(port, UART_LCR, lcr);
> +
> + quot = (dlm << 8) | dll;
> + return (port->uartclk / 16) / quot;
> +}
> +
You said some "newer" chips do not support probe baud. why do you move
code to core ?
I was thinking some embedded guys could comment out 8280_early.c, now you get
extra work for them to comment out code from 8250_core.c.
> /**
> * univ8250_console_match - non-standard console matching
> * @co: registering console
> @@ -3455,13 +3470,18 @@ static int univ8250_console_setup(struct console *co, char *options)
> * @options: ptr to option string from console command line
> *
> * Only attempts to match console command lines of the form:
> - * console=uart<>,io|mmio|mmio32,<addr>,<options>
> - * console=uart<>,<addr>,options
> + * console=uart[8250],io|mmio|mmio32,<addr>[,<options>]
> + * console=uart[8250],0x<addr>[,<options>]
> * This form is used to register an initial earlycon boot console and
> * replace it with the serial8250_console at 8250 driver init.
> *
> * Performs console setup for a match (as required by interface)
> *
> + * ** HACK ALERT **
That is not HACK original.
but your fix make it to be hack.
> + * If no <options> are specified, then assume the h/w is already setup.
> + * This was the undocumented behavior of the original implementation so
> + * it is cast in stone forever.
> + *
> * Returns 0 if console matches; otherwise non-zero to use default matching
> */
> static int univ8250_console_match(struct console *co, char *name, int idx,
> @@ -3475,12 +3495,16 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
> if (strncmp(name, match, 4) != 0)
> return -ENODEV;
>
> + if (idx && idx != 8250)
> + return -ENODEV;
> +
Your fix is getting ugly now.
> if (uart_parse_earlycon(options, &iotype, &addr, &options))
> return -ENODEV;
>
> /* try to match the port specified on the command line */
> for (i = 0; i < nr_uarts; i++) {
> struct uart_port *port = &serial8250_ports[i].port;
> + int baud, bits = 8, parity = 'n', flow = 'n';
>
> if (port->iotype != iotype)
> continue;
> @@ -3490,8 +3514,15 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
> if (iotype == UPIO_PORT && port->iobase != addr)
> continue;
>
> + /* link port to console */
> co->index = i;
> - return univ8250_console_setup(co, options);
> + port->cons = co;
> +
> + if (options && *options)
> + uart_parse_options(options, &baud, &parity, &bits, &flow);
> + else
> + baud = probe_baud(port);
> + return uart_set_options(port, port->cons, baud, parity, bits, flow);
what a mess.
Now sure if it is safe to move down probe_baud, when serial port is still used.
> }
>
> return -ENODEV;
> diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
> index e95ebfe..8e11968 100644
> --- a/drivers/tty/serial/8250/8250_early.c
> +++ b/drivers/tty/serial/8250/8250_early.c
> @@ -105,21 +105,6 @@ static void __init early_serial8250_write(struct console *console,
> serial8250_early_out(port, UART_IER, ier);
> }
>
> -static unsigned int __init probe_baud(struct uart_port *port)
> -{
> - unsigned char lcr, dll, dlm;
> - unsigned int quot;
> -
> - lcr = serial8250_early_in(port, UART_LCR);
> - serial8250_early_out(port, UART_LCR, lcr | UART_LCR_DLAB);
> - dll = serial8250_early_in(port, UART_DLL);
> - dlm = serial8250_early_in(port, UART_DLM);
> - serial8250_early_out(port, UART_LCR, lcr);
> -
> - quot = (dlm << 8) | dll;
> - return (port->uartclk / 16) / quot;
> -}
> -
> static void __init init_port(struct earlycon_device *device)
> {
> struct uart_port *port = &device->port;
> @@ -151,10 +136,6 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
> struct uart_port *port = &device->port;
> unsigned int ier;
>
> - device->baud = probe_baud(&device->port);
> - snprintf(device->options, sizeof(device->options), "%u",
> - device->baud);
> -
> /* assume the device was initialized, only mask interrupts */
> ier = serial8250_early_in(port, UART_IER);
> serial8250_early_out(port, UART_IER, ier & UART_IER_UUE);
> --
Greg, Please consider apply my fix as attached, it is much cleaner.
Or just drop commit c7cef0a84912 ("console: Add extensible console matching")
from the tty-next now.
Thanks
Yinghai
View attachment "fix_earlycon_console_handover_v3.patch" of type "text/x-patch" (3884 bytes)
Powered by blists - more mailing lists