linux-kernel - Re: [PATCH] tty/serial: emit CR before NL in RISC-V SBL console

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 10 Jan 2019 12:54:44 -0800 (PST)
From:   Palmer Dabbelt <palmer@...ive.com>
To:     schwab@...e.de, anup@...infault.org
CC:     Greg KH <gregkh@...uxfoundation.org>, jslaby@...e.com,
        aou@...s.berkeley.edu, atish.patra@....com,
        Christoph Hellwig <hch@...radead.org>, robh@...nel.org,
        linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-serial@...r.kernel.org
Subject:     Re: [PATCH] tty/serial: emit CR before NL in RISC-V SBL console

On Thu, 10 Jan 2019 07:26:46 PST (-0800), schwab@...e.de wrote:
> On Jan 10 2019, Anup Patel <anup@...infault.org> wrote:
>
>> Instead of doing '\n' handling here, we should do it in BBL or
>> OpenSBI (i.e. SBI runtime firmware) otherwise all users of
>> SBI_CONSOLE_PUTCHAR (namely, Linux, FreeBSD,
>> FreeRTOS, U-Boot S-mode, etc) will endup having '\n'
>> handling.
>
> I don't think the serial driver should do NLCR handling on its own,
> without being instructed by the tty layer.  Since the earlycon does not
> have a tty layer, it needs to handle it explicitly.

I think it's best if we have the SBI interfaces be defined to be bit-exact.  It
does mean that everyone who uses the interfaces needs to do this handling, but
I think that's actually a feature because it'll be subtly different everywhere.

This also has the advantage of being compatible with what we currently do in
BBL and what we current expect in Linux everywhere but this earlycon driver
(and will do assuming we merge this patch).

On Thu, 10 Jan 2019 08:17:07 PST (-0800), anup@...infault.org wrote:
> On Thu, Jan 10, 2019 at 8:56 PM Andreas Schwab <schwab@...e.de> wrote:
>>
>> On Jan 10 2019, Anup Patel <anup@...infault.org> wrote:
>>
>> > Instead of doing '\n' handling here, we should do it in BBL or
>> > OpenSBI (i.e. SBI runtime firmware) otherwise all users of
>> > SBI_CONSOLE_PUTCHAR (namely, Linux, FreeBSD,
>> > FreeRTOS, U-Boot S-mode, etc) will endup having '\n'
>> > handling.
>>
>> I don't think the serial driver should do NLCR handling on its own,
>> without being instructed by the tty layer.  Since the earlycon does not
>> have a tty layer, it needs to handle it explicitly.
>
> I tried to investigate more. What you suggest is correct
> but we should use uart_console_write() API.
>
> Instead of explicitly doing NLCR here, we should do
> something as follows:
>
> static void sbi_putc(struct uart_port *port, int c)
> {
>     sbi_console_putchar(c);
> }
>
> static void sbi_console_write(struct console *con,
>                                                const char *s, unsigned n)
> {
>     struct earlycon_device *dev = con->data;
>     uart_console_write(&dev->port, s, n, sbi_putc);
> }
>
> The uart_console_write() does the NLCR handling.

That does look cleaner.  I'll expect a v2 :)