* [PATCH v2 0/2] serial: sifive: Convert sifive console to nbcon @ 2025-03-30 0:30 Ryo Takakura 2025-03-30 0:35 ` [PATCH v2 1/2] serial: sifive: lock port in startup()/shutdown() callbacks Ryo Takakura 2025-03-30 0:40 ` [PATCH v2 2/2] serial: sifive: Switch to nbcon console Ryo Takakura 0 siblings, 2 replies; 6+ messages in thread From: Ryo Takakura @ 2025-03-30 0:30 UTC (permalink / raw) To: alex, aou, gregkh, jirislaby, john.ogness, palmer, paul.walmsley, pmladek, samuel.holland, bigeasy, conor.dooley, u.kleine-koenig Cc: linux-kernel, linux-riscv, linux-serial, Ryo Takakura Hi! This series convert sifive console to nbcon. The first patch fixes the issue which was pointed out by John [0] that the driver has been accessing SIFIVE_SERIAL_IE_OFFS register on its ->startup() and ->shutdown() without port lock synchronization against ->write(). The fix on the first patch still applies to the second patch which converts the console to nbcon as ->write_thread() holds port lock and ->write_atomic() checks for the console ownership. Sincerely, Ryo Takakura [0] https://lore.kernel.org/lkml/84sen2fo4b.fsf@jogness.linutronix.de/ --- Changes since v1: [1] https://lore.kernel.org/lkml/20250323060603.388621-1-ryotkkr98@gmail.com/ - Thank you John for the feedback! - Add a patch for synchronizing startup()/shutdown() vs write(). - Add <Reviewed-by> by John. --- Ryo Takakura (2): serial: sifive: lock port in startup()/shutdown() callbacks serial: sifive: Switch to nbcon console drivers/tty/serial/sifive.c | 93 +++++++++++++++++++++++++++++++------ 1 file changed, 80 insertions(+), 13 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] serial: sifive: lock port in startup()/shutdown() callbacks 2025-03-30 0:30 [PATCH v2 0/2] serial: sifive: Convert sifive console to nbcon Ryo Takakura @ 2025-03-30 0:35 ` Ryo Takakura 2025-03-30 0:40 ` [PATCH v2 2/2] serial: sifive: Switch to nbcon console Ryo Takakura 1 sibling, 0 replies; 6+ messages in thread From: Ryo Takakura @ 2025-03-30 0:35 UTC (permalink / raw) To: alex, aou, gregkh, jirislaby, john.ogness, palmer, paul.walmsley, pmladek, samuel.holland, bigeasy, conor.dooley, u.kleine-koenig Cc: linux-kernel, linux-riscv, linux-serial, stable, Ryo Takakura startup()/shutdown() callbacks access SIFIVE_SERIAL_IE_OFFS. The register is also accessed from write() callback. If console were printing and startup()/shutdown() callback gets called, its access to the register could be overwritten. Add port->lock to startup()/shutdown() callbacks to make sure their access to SIFIVE_SERIAL_IE_OFFS is synchronized against write() callback. Signed-off-by: Ryo Takakura <ryotkkr98@gmail.com> --- drivers/tty/serial/sifive.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c index 5904a2d4c..054a8e630 100644 --- a/drivers/tty/serial/sifive.c +++ b/drivers/tty/serial/sifive.c @@ -563,8 +563,11 @@ static void sifive_serial_break_ctl(struct uart_port *port, int break_state) static int sifive_serial_startup(struct uart_port *port) { struct sifive_serial_port *ssp = port_to_sifive_serial_port(port); + unsigned long flags; + uart_port_lock_irqsave(&ssp->port, &flags); __ssp_enable_rxwm(ssp); + uart_port_unlock_irqrestore(&ssp->port, flags); return 0; } @@ -572,9 +575,12 @@ static int sifive_serial_startup(struct uart_port *port) static void sifive_serial_shutdown(struct uart_port *port) { struct sifive_serial_port *ssp = port_to_sifive_serial_port(port); + unsigned long flags; + uart_port_lock_irqsave(&ssp->port, &flags); __ssp_disable_rxwm(ssp); __ssp_disable_txwm(ssp); + uart_port_unlock_irqrestore(&ssp->port, flags); } /** -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] serial: sifive: Switch to nbcon console 2025-03-30 0:30 [PATCH v2 0/2] serial: sifive: Convert sifive console to nbcon Ryo Takakura 2025-03-30 0:35 ` [PATCH v2 1/2] serial: sifive: lock port in startup()/shutdown() callbacks Ryo Takakura @ 2025-03-30 0:40 ` Ryo Takakura 1 sibling, 0 replies; 6+ messages in thread From: Ryo Takakura @ 2025-03-30 0:40 UTC (permalink / raw) To: alex, aou, gregkh, jirislaby, john.ogness, palmer, paul.walmsley, pmladek, samuel.holland, bigeasy, conor.dooley, u.kleine-koenig Cc: linux-kernel, linux-riscv, linux-serial, Ryo Takakura Add the necessary callbacks(write_atomic, write_thread, device_lock and device_unlock) and CON_NBCON flag to switch the sifive console driver to perform as nbcon console. Both ->write_atomic() and ->write_thread() will check for console ownership whenever they are accessing registers. The ->device_lock()/unlock() will provide the additional serilization necessary for ->write_thread() which is called from dedicated printing thread. Signed-off-by: Ryo Takakura <ryotkkr98@gmail.com> Reviewed-by: John Ogness <john.ogness@linutronix.de> --- drivers/tty/serial/sifive.c | 87 +++++++++++++++++++++++++++++++------ 1 file changed, 74 insertions(+), 13 deletions(-) diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c index 054a8e630..37d5820af 100644 --- a/drivers/tty/serial/sifive.c +++ b/drivers/tty/serial/sifive.c @@ -151,6 +151,7 @@ struct sifive_serial_port { unsigned long baud_rate; struct clk *clk; struct notifier_block clk_notifier; + bool console_line_ended; }; /* @@ -785,33 +786,88 @@ static void sifive_serial_console_putchar(struct uart_port *port, unsigned char __ssp_wait_for_xmitr(ssp); __ssp_transmit_char(ssp, ch); + + ssp->console_line_ended = (ch == '\n'); +} + +static void sifive_serial_device_lock(struct console *co, unsigned long *flags) +{ + struct uart_port *up = &sifive_serial_console_ports[co->index]->port; + + return __uart_port_lock_irqsave(up, flags); +} + +static void sifive_serial_device_unlock(struct console *co, unsigned long flags) +{ + struct uart_port *up = &sifive_serial_console_ports[co->index]->port; + + return __uart_port_unlock_irqrestore(up, flags); } -static void sifive_serial_console_write(struct console *co, const char *s, - unsigned int count) +static void sifive_serial_console_write_atomic(struct console *co, + struct nbcon_write_context *wctxt) { struct sifive_serial_port *ssp = sifive_serial_console_ports[co->index]; - unsigned long flags; + struct uart_port *port = &ssp->port; unsigned int ier; - int locked = 1; if (!ssp) return; - if (oops_in_progress) - locked = uart_port_trylock_irqsave(&ssp->port, &flags); - else - uart_port_lock_irqsave(&ssp->port, &flags); + if (!nbcon_enter_unsafe(wctxt)) + return; ier = __ssp_readl(ssp, SIFIVE_SERIAL_IE_OFFS); __ssp_writel(0, SIFIVE_SERIAL_IE_OFFS, ssp); - uart_console_write(&ssp->port, s, count, sifive_serial_console_putchar); + if (!ssp->console_line_ended) + uart_console_write(port, "\n", 1, sifive_serial_console_putchar); + uart_console_write(port, wctxt->outbuf, wctxt->len, + sifive_serial_console_putchar); __ssp_writel(ier, SIFIVE_SERIAL_IE_OFFS, ssp); - if (locked) - uart_port_unlock_irqrestore(&ssp->port, flags); + nbcon_exit_unsafe(wctxt); +} + +static void sifive_serial_console_write_thread(struct console *co, + struct nbcon_write_context *wctxt) +{ + struct sifive_serial_port *ssp = sifive_serial_console_ports[co->index]; + struct uart_port *port = &ssp->port; + unsigned int ier; + + if (!ssp) + return; + + if (!nbcon_enter_unsafe(wctxt)) + return; + + ier = __ssp_readl(ssp, SIFIVE_SERIAL_IE_OFFS); + __ssp_writel(0, SIFIVE_SERIAL_IE_OFFS, ssp); + + if (nbcon_exit_unsafe(wctxt)) { + int len = READ_ONCE(wctxt->len); + int i; + + for (i = 0; i < len; i++) { + if (!nbcon_enter_unsafe(wctxt)) + break; + + uart_console_write(port, wctxt->outbuf + i, 1, + sifive_serial_console_putchar); + + if (!nbcon_exit_unsafe(wctxt)) + break; + } + } + + while (!nbcon_enter_unsafe(wctxt)) + nbcon_reacquire_nobuf(wctxt); + + __ssp_writel(ier, SIFIVE_SERIAL_IE_OFFS, ssp); + + nbcon_exit_unsafe(wctxt); } static int sifive_serial_console_setup(struct console *co, char *options) @@ -829,6 +885,8 @@ static int sifive_serial_console_setup(struct console *co, char *options) if (!ssp) return -ENODEV; + ssp->console_line_ended = true; + if (options) uart_parse_options(options, &baud, &parity, &bits, &flow); @@ -839,10 +897,13 @@ static struct uart_driver sifive_serial_uart_driver; static struct console sifive_serial_console = { .name = SIFIVE_TTY_PREFIX, - .write = sifive_serial_console_write, + .write_atomic = sifive_serial_console_write_atomic, + .write_thread = sifive_serial_console_write_thread, + .device_lock = sifive_serial_device_lock, + .device_unlock = sifive_serial_device_unlock, .device = uart_console_device, .setup = sifive_serial_console_setup, - .flags = CON_PRINTBUFFER, + .flags = CON_PRINTBUFFER | CON_NBCON, .index = -1, .data = &sifive_serial_uart_driver, }; -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
[parent not found: <Z-iSb0ryR-tiUCj0@42be267012b8>]
* Re: [PATCH v2 1/2] serial: sifive: lock port in startup()/shutdown() callbacks [not found] <Z-iSb0ryR-tiUCj0@42be267012b8> @ 2025-03-30 1:16 ` Ryo Takakura 2025-03-30 7:30 ` Greg KH 0 siblings, 1 reply; 6+ messages in thread From: Ryo Takakura @ 2025-03-30 1:16 UTC (permalink / raw) To: alex, aou, gregkh, jirislaby, john.ogness, palmer, paul.walmsley, pmladek, samuel.holland, bigeasy, conor.dooley, u.kleine-koenig, lkp Cc: linux-kernel, linux-riscv, linux-serial, stable, oe-kbuild-all, Ryo Takakura startup()/shutdown() callbacks access SIFIVE_SERIAL_IE_OFFS. The register is also accessed from write() callback. If console were printing and startup()/shutdown() callback gets called, its access to the register could be overwritten. Add port->lock to startup()/shutdown() callbacks to make sure their access to SIFIVE_SERIAL_IE_OFFS is synchronized against write() callback. Signed-off-by: Ryo Takakura <ryotkkr98@gmail.com> Cc: stable@vger.kernel.org --- Hi, I'm sorry that I wasn't aware of how Cc stable should be done. I added Cc for stable but please tell me if this patch should be resent or if there is any that is missing. Sincerely, Ryo Takakura --- drivers/tty/serial/sifive.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c index 5904a2d4c..054a8e630 100644 --- a/drivers/tty/serial/sifive.c +++ b/drivers/tty/serial/sifive.c @@ -563,8 +563,11 @@ static void sifive_serial_break_ctl(struct uart_port *port, int break_state) static int sifive_serial_startup(struct uart_port *port) { struct sifive_serial_port *ssp = port_to_sifive_serial_port(port); + unsigned long flags; + uart_port_lock_irqsave(&ssp->port, &flags); __ssp_enable_rxwm(ssp); + uart_port_unlock_irqrestore(&ssp->port, flags); return 0; } @@ -572,9 +575,12 @@ static int sifive_serial_startup(struct uart_port *port) static void sifive_serial_shutdown(struct uart_port *port) { struct sifive_serial_port *ssp = port_to_sifive_serial_port(port); + unsigned long flags; + uart_port_lock_irqsave(&ssp->port, &flags); __ssp_disable_rxwm(ssp); __ssp_disable_txwm(ssp); + uart_port_unlock_irqrestore(&ssp->port, flags); } /** -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] serial: sifive: lock port in startup()/shutdown() callbacks 2025-03-30 1:16 ` [PATCH v2 1/2] serial: sifive: lock port in startup()/shutdown() callbacks Ryo Takakura @ 2025-03-30 7:30 ` Greg KH 2025-03-30 10:51 ` Ryo Takakura 0 siblings, 1 reply; 6+ messages in thread From: Greg KH @ 2025-03-30 7:30 UTC (permalink / raw) To: Ryo Takakura Cc: alex, aou, jirislaby, john.ogness, palmer, paul.walmsley, pmladek, samuel.holland, bigeasy, conor.dooley, u.kleine-koenig, lkp, linux-kernel, linux-riscv, linux-serial, stable, oe-kbuild-all On Sun, Mar 30, 2025 at 10:16:10AM +0900, Ryo Takakura wrote: > startup()/shutdown() callbacks access SIFIVE_SERIAL_IE_OFFS. > The register is also accessed from write() callback. > > If console were printing and startup()/shutdown() callback > gets called, its access to the register could be overwritten. > > Add port->lock to startup()/shutdown() callbacks to make sure > their access to SIFIVE_SERIAL_IE_OFFS is synchronized against > write() callback. > > Signed-off-by: Ryo Takakura <ryotkkr98@gmail.com> > Cc: stable@vger.kernel.org > --- > > Hi, > > I'm sorry that I wasn't aware of how Cc stable should be done. > > I added Cc for stable but please tell me if this patch should be > resent or if there is any that is missing. Please resend a v3. thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] serial: sifive: lock port in startup()/shutdown() callbacks 2025-03-30 7:30 ` Greg KH @ 2025-03-30 10:51 ` Ryo Takakura 0 siblings, 0 replies; 6+ messages in thread From: Ryo Takakura @ 2025-03-30 10:51 UTC (permalink / raw) To: gregkh Cc: alex, aou, bigeasy, conor.dooley, jirislaby, john.ogness, linux-kernel, linux-riscv, linux-serial, lkp, oe-kbuild-all, palmer, paul.walmsley, pmladek, ryotkkr98, samuel.holland, stable, u.kleine-koenig Hi Greg, On Sun, 30 Mar 2025 09:30:27 +0200, Greg KH wrote: >On Sun, Mar 30, 2025 at 10:16:10AM +0900, Ryo Takakura wrote: >> startup()/shutdown() callbacks access SIFIVE_SERIAL_IE_OFFS. >> The register is also accessed from write() callback. >> >> If console were printing and startup()/shutdown() callback >> gets called, its access to the register could be overwritten. >> >> Add port->lock to startup()/shutdown() callbacks to make sure >> their access to SIFIVE_SERIAL_IE_OFFS is synchronized against >> write() callback. >> >> Signed-off-by: Ryo Takakura <ryotkkr98@gmail.com> >> Cc: stable@vger.kernel.org >> --- >> >> Hi, >> >> I'm sorry that I wasn't aware of how Cc stable should be done. >> >> I added Cc for stable but please tell me if this patch should be >> resent or if there is any that is missing. > >Please resend a v3. Ok. I'll send v3 shortly ;) Sincerely, Ryo Takakura >thanks, > >greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-03-30 10:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-30 0:30 [PATCH v2 0/2] serial: sifive: Convert sifive console to nbcon Ryo Takakura
2025-03-30 0:35 ` [PATCH v2 1/2] serial: sifive: lock port in startup()/shutdown() callbacks Ryo Takakura
2025-03-30 0:40 ` [PATCH v2 2/2] serial: sifive: Switch to nbcon console Ryo Takakura
[not found] <Z-iSb0ryR-tiUCj0@42be267012b8>
2025-03-30 1:16 ` [PATCH v2 1/2] serial: sifive: lock port in startup()/shutdown() callbacks Ryo Takakura
2025-03-30 7:30 ` Greg KH
2025-03-30 10:51 ` Ryo Takakura
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).