linux-kernel - Re: [PATCH RESEND v6 0/3] serial: 8250

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]
Message-ID: <CAHp75VfPEdxN1UeKJ+gCWpgJymK7YzQs1Lznq1aBfoRNBiMHbQ@mail.gmail.com>
Date:   Thu, 18 Jun 2020 11:17:47 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Serge Semin <Sergey.Semin@...kalelectronics.ru>,
        Tony Lindgren <tony@...mide.com>
Cc:     Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>,
        Serge Semin <fancer.lancer@...il.com>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Maxim Kaurkin <Maxim.Kaurkin@...kalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>,
        Alexey Kolotnikov <Alexey.Kolotnikov@...kalelectronics.ru>,
        Ramil Zaripov <Ramil.Zaripov@...kalelectronics.ru>,
        Ekaterina Skachko <Ekaterina.Skachko@...kalelectronics.ru>,
        Vadim Vlasov <V.Vlasov@...kalelectronics.ru>,
        Arnd Bergmann <arnd@...db.de>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Will Deacon <will@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        linux-mips@...r.kernel.org,
        linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
        "open list:SERIAL DRIVERS" <linux-serial@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RESEND v6 0/3] serial: 8250_dw: Fix ref clock usage

On Thu, Jun 18, 2020 at 1:52 AM Serge Semin
<Sergey.Semin@...kalelectronics.ru> wrote:
>
> Greg, Jiri. We've missed the last merge window. It would be pity to miss
> the next one. Please review/merge in the series.
>
> Regarding the patchset. It might be dangerous if an UART port reference
> clock rate is suddenly changed. In particular the 8250 port drivers
> (and AFAICS most of the tty drivers using common clock framework clocks)
> rely either on the exclusive reference clock utilization or on the ref
> clock rate being always constant. Needless to say that it turns out not
> true and if some other service suddenly changes the clock rate behind an
> UART port driver back no good can happen. So the port might not only end
> up with an invalid uartclk value saved, but may also experience a
> distorted output/input data since such action will effectively update the
> programmed baud-clock. We discovered such problem on Baikal-T1 SoC where
> two DW 8250 ports have got a shared reference clock. Allwinner SoC is
> equipped with an UART, which clock is derived from the CPU PLL clock
> source, so the CPU frequency change might be propagated down up to the
> serial port reference clock. This patchset provides a way to fix the
> problem to the 8250 serial port controllers and mostly fixes it for the
> DW 8250-compatible UART. I say mostly because due to not having a facility
> to pause/stop and resume/restart on-going transfers we implemented the
> UART clock rate update procedure executed post factum of the actual
> reference clock rate change.
>
> In addition the patchset includes a small optimization patch. It
> simplifies the DW APB UART ref clock rate setting procedure a bit.
>
> This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4:
> base-commit: 0e698dfa2822 ("Linux 5.7-rc4")
> tag: v5.7-rc4

I'm wondering how this will collaborate with runtime PM.

> Changelog v3:
> - Refactor the original patch to adjust the UART port divisor instead of
>   requesting an exclusive ref clock utilization.
>
> Changelog v4:
> - Discard commit b426bf0fb085 ("serial: 8250: Fix max baud limit in generic
>   8250 port") since Greg has already merged it into the tty-next branch.
> - Use EXPORT_SYMBOL_GPL() for the serial8250_update_uartclk() method.
>
> Changelog v5:
> - Refactor dw8250_clk_work_cb() function cheking the clk_get_rate()
>   return value for being erroneous and exit if it is.
> - Don't update p->uartclk in the port startup. It will be updated later in
>   the same procedure at the set_termios() function being invoked by the
>   serial_core anyway.
>
> Changelog v6:
> - Resend
>
> Signed-off-by: Serge Semin <Sergey.Semin@...kalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>
> Cc: Maxim Kaurkin <Maxim.Kaurkin@...kalelectronics.ru>
> Cc: Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>
> Cc: Alexey Kolotnikov <Alexey.Kolotnikov@...kalelectronics.ru>
> Cc: Ramil Zaripov <Ramil.Zaripov@...kalelectronics.ru>
> Cc: Ekaterina Skachko <Ekaterina.Skachko@...kalelectronics.ru>
> Cc: Vadim Vlasov <V.Vlasov@...kalelectronics.ru>
> Cc: Alexey Kolotnikov <Alexey.Kolotnikov@...kalelectronics.ru>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Cc: Maxime Ripard <mripard@...nel.org>
> Cc: Will Deacon <will@...nel.org>
> Cc: Russell King <linux@...linux.org.uk>
> Cc: linux-mips@...r.kernel.org
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-serial@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
>
> Serge Semin (3):
>   serial: 8250: Add 8250 port clock update method
>   serial: 8250_dw: Simplify the ref clock rate setting procedure
>   serial: 8250_dw: Fix common clocks usage race condition
>
>  drivers/tty/serial/8250/8250_dw.c   | 116 +++++++++++++++++++++++++---
>  drivers/tty/serial/8250/8250_port.c |  38 +++++++++
>  include/linux/serial_8250.h         |   2 +
>  3 files changed, 144 insertions(+), 12 deletions(-)
>
> --
> 2.26.2
>


-- 
With Best Regards,
Andy Shevchenko