* [PATCH printk v1 00/18] threaded/atomic console support @ 2023-03-02 19:56 John Ogness 2023-03-02 19:56 ` [PATCH printk v1 17/18] rcu: Add atomic write enforcement for rcu stalls John Ogness ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: John Ogness @ 2023-03-02 19:56 UTC (permalink / raw) To: Petr Mladek Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Jason Wessel, Daniel Thompson, Douglas Anderson, Aaron Tomlin, Luis Chamberlain, kgdb-bugreport, Greg Kroah-Hartman, linux-fsdevel, Andrew Morton, Guilherme G. Piccoli, David Gow, Tiezhu Yang, Daniel Vetter, tangmeng, Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes, rcu Hi, This is v1 of a series to bring in a new threaded/atomic console infrastructure. The history, motivation, and various explanations and examples are available in the cover letter of tglx's RFC series [0]. From that series, patches 1-18 have been mainlined as of the 6.3 merge window. What remains, patches 19-29, is what this series represents. Since the RFC there has been significant changes involving bug-fixing, refactoring, renaming, and feature completion. Despite the many changes in the code, the concept and design has been preserved. The key points to the threaded/atomic (aka NOBKL) consoles are: - Each console has its own kthread and uses a new write_thread() console callback that is always called from sleepable context. The kthreads of different consoles do not contend with each other and do not use the global console lock. - Each console uses a new write_atomic() console callback that is able to write from any context (including NMI). The threaded/atomic infrastructure provides supporting functions to help atomic console drivers to synchronize with their own threaded and re-entrant atomic counterparts. - Until the console threads are available, atomic printing is performed. The threaded printing is able to take over shortly before SMP is brought online so machines with many CPUs should be able to boot at full speed without being held back by console printing. - When console threads are shutdown (on system reboot and shutdown), again the atomic consoles take over. This ensures the final message make it out to the consoles. - When panic, WARN, and NMI stall detection occurs, the atomic consoles temporarily take over printing until the related messages have been output. In this case the full set of related messages are stored into the printk ringbuffer before the atomic consoles begin flushing the ringbuffer to the consoles. - Atomic printing is split into 3 priorities. For example, upon shutdown (when kthreads are not available), the console output will be normal priority atomic printing. This could be interrupted by a WARN on another CPU (emergency priority). And that could be interrupted by a panic on yet another CPU (panic priority). And of course any atomic printing priority can interrupt the kthread printer. - The transition between kthread and any atomic printing or to any elevated priority atomic printing is synchronized using an atomic_t state variable per console. The state variable acts like a spinlock but with special properties that the spinning can timeout and even a hostile takeover based on the atomic priorities is possible. After outputting each byte, a console printing context checks the state variable to ensure it is still the owner of the console. If it is not (for example, in the case of a hostile takeover) it will immediately abort any continued use of the console and rely on the new owner to flush the messages. - Using the console state variable, console drivers can mark unsafe regions in their code where ownership transition is not possible. Combined with the timeout feature, a handover protocol, and the possibility for a hostile takeover, this allows drivers to make safe decisions about when and how console ownership is transferred to another context. It also allows the printk infrastructure to make safe decisions in panic situations, such as only outputting to atomic consoles where safe takeovers are possible. And only after handling all other panic responsibilities, attempting unsafe takeovers for the consoles that have not yet transferred ownership. - In order to support hostile takeovers (where a CPU with a higher priority context can steal ownership from another CPU) without CPUs clobbering each others buffers, each CPU has its own set of string print buffers. The existing legacy consoles continue to function unmodified as before and legacy consoles can work next to NOBKL consoles (i.e. a legacy virtual terminal graphics console and network console will work with a NOBKL uart8250 console). However, in order to have the full benefit/reliability of NOBKL consoles, a system should use _only_ NOBKL consoles. We believe that this series covers all printk features and usage to allow new threaded/atomic consoles to be able to replace the legacy consoles. However, this will be a gradual transition as individual console drivers are updated to support the NOBKL requirements. This series does not include any changes to console drivers to allow them to act as NOBKL consoles. That will be a follow-up series, once a finalized infrastructure is in place. However, I will reply to this message with an all-in-one uart8250 patch that fully implements NOBKL support. The patch will allow you to perform runtime tests with the NOBKL consoles on the uart8250. John Ogness [0] https://lore.kernel.org/lkml/20220910221947.171557773@linutronix.de John Ogness (7): kdb: do not assume write() callback available printk: Add NMI check to down_trylock_console_sem() printk: Consolidate console deferred printing printk: Add per-console suspended state printk: nobkl: Stop threads on shutdown/reboot rcu: Add atomic write enforcement for rcu stalls printk: Perform atomic flush in console_flush_on_panic() Thomas Gleixner (11): printk: Add non-BKL console basic infrastructure printk: nobkl: Add acquire/release logic printk: nobkl: Add buffer management printk: nobkl: Add sequence handling printk: nobkl: Add print state functions printk: nobkl: Add emit function and callback functions for atomic printing printk: nobkl: Introduce printer threads printk: nobkl: Add printer thread wakeups printk: nobkl: Add write context storage for atomic writes printk: nobkl: Provide functions for atomic write enforcement kernel/panic: Add atomic write enforcement to warn/panic fs/proc/consoles.c | 1 + include/linux/console.h | 174 ++++ include/linux/printk.h | 9 + kernel/debug/kdb/kdb_io.c | 2 + kernel/panic.c | 17 + kernel/printk/Makefile | 2 +- kernel/printk/internal.h | 103 +- kernel/printk/printk.c | 307 ++++-- kernel/printk/printk_nobkl.c | 1820 ++++++++++++++++++++++++++++++++++ kernel/printk/printk_safe.c | 9 +- kernel/rcu/tree_stall.h | 6 + 11 files changed, 2362 insertions(+), 88 deletions(-) create mode 100644 kernel/printk/printk_nobkl.c base-commit: 10d639febe5629687dac17c4a7500a96537ce11a -- 2.30.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH printk v1 17/18] rcu: Add atomic write enforcement for rcu stalls 2023-03-02 19:56 [PATCH printk v1 00/18] threaded/atomic console support John Ogness @ 2023-03-02 19:56 ` John Ogness 2023-04-13 12:10 ` Petr Mladek 2023-03-02 19:58 ` [PATCH printk v1 00/18] serial: 8250: implement non-BKL console John Ogness 2023-03-09 10:55 ` [PATCH printk v1 00/18] threaded/atomic console support Daniel Thompson 2 siblings, 1 reply; 12+ messages in thread From: John Ogness @ 2023-03-02 19:56 UTC (permalink / raw) To: Petr Mladek Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes, rcu Invoke the atomic write enforcement functions for rcu stalls to ensure that the information gets out to the consoles. It is important to note that if there are any legacy consoles registered, they will be attempting to directly print from the printk-caller context, which may jeopardize the reliability of the atomic consoles. Optimally there should be no legacy consoles registered. Signed-off-by: John Ogness <john.ogness@linutronix.de> --- kernel/rcu/tree_stall.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h index 5653560573e2..25207a213e7a 100644 --- a/kernel/rcu/tree_stall.h +++ b/kernel/rcu/tree_stall.h @@ -8,6 +8,7 @@ */ #include <linux/kvm_para.h> +#include <linux/console.h> ////////////////////////////////////////////////////////////////////////////// // @@ -551,6 +552,7 @@ static void rcu_check_gp_kthread_expired_fqs_timer(void) static void print_other_cpu_stall(unsigned long gp_seq, unsigned long gps) { + enum cons_prio prev_prio; int cpu; unsigned long flags; unsigned long gpa; @@ -566,6 +568,8 @@ static void print_other_cpu_stall(unsigned long gp_seq, unsigned long gps) if (rcu_stall_is_suppressed()) return; + prev_prio = cons_atomic_enter(CONS_PRIO_EMERGENCY); + /* * OK, time to rat on our buddy... * See Documentation/RCU/stallwarn.rst for info on how to debug @@ -620,6 +624,8 @@ static void print_other_cpu_stall(unsigned long gp_seq, unsigned long gps) panic_on_rcu_stall(); rcu_force_quiescent_state(); /* Kick them all. */ + + cons_atomic_exit(CONS_PRIO_EMERGENCY, prev_prio); } static void print_cpu_stall(unsigned long gps) -- 2.30.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH printk v1 17/18] rcu: Add atomic write enforcement for rcu stalls 2023-03-02 19:56 ` [PATCH printk v1 17/18] rcu: Add atomic write enforcement for rcu stalls John Ogness @ 2023-04-13 12:10 ` Petr Mladek 0 siblings, 0 replies; 12+ messages in thread From: Petr Mladek @ 2023-04-13 12:10 UTC (permalink / raw) To: John Ogness Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes, rcu On Thu 2023-03-02 21:02:17, John Ogness wrote: > Invoke the atomic write enforcement functions for rcu stalls to > ensure that the information gets out to the consoles. "ensure" is too strong. It is still just the best effort. It might fail when the current console user does not pass the lock. I would say that it will increase the chance to see the messages on NOBKL consoles by printing the messages directly instead of waiting for the printk thread. > It is important to note that if there are any legacy consoles > registered, they will be attempting to directly print from the > printk-caller context, which may jeopardize the reliability of the > atomic consoles. Optimally there should be no legacy consoles > registered. The above paragraph is a bit vague. It is not clear how exactly the legacy consoles affect the reliability, Does it mean that they might cause a deadlock because they are not atomic? But there is nothing specific about rcu stalls and priority of NOBKL consoles. This is a generic problem with legacy consoles. > --- a/kernel/rcu/tree_stall.h > +++ b/kernel/rcu/tree_stall.h > @@ -566,6 +568,8 @@ static void print_other_cpu_stall(unsigned long gp_seq, unsigned long gps) > if (rcu_stall_is_suppressed()) > return; > > + prev_prio = cons_atomic_enter(CONS_PRIO_EMERGENCY); Thinking loudly: This would set the EMERGENCY priority on this CPU. But the following function: + rcu_dump_cpu_stacks() + dump_cpu_task() + trigger_single_cpu_backtrace() might send IPI and the backtrace will be printed from another CPU. As a result that backtraces won't be printed with EMERGENCY priority. One solution would be to have also global EMERGENCY priority. Another possibility would be to use EMERGENCY priority also in nmi_cpu_backtrace() which is the callback called by the IPI. I would probably go for the global flag. printk() called in EMERGENCY priority has to flush also messages added by other CPUs. So that messages added by other CPUs are printed "directly" anyway. Also setting the EMERGENCY priority in nmi_cpu_backtrace() is an ad-hoc solution. The backtrace is usually called as part of another global emergency report. > + > /* > * OK, time to rat on our buddy... > * See Documentation/RCU/stallwarn.rst for info on how to debug Best Regards, Petr ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH printk v1 00/18] serial: 8250: implement non-BKL console 2023-03-02 19:56 [PATCH printk v1 00/18] threaded/atomic console support John Ogness 2023-03-02 19:56 ` [PATCH printk v1 17/18] rcu: Add atomic write enforcement for rcu stalls John Ogness @ 2023-03-02 19:58 ` John Ogness 2023-03-28 13:33 ` locking API: was: " Petr Mladek 2023-03-28 13:59 ` [PATCH printk v1 00/18] POC: serial: 8250: implement nbcon console John Ogness 2023-03-09 10:55 ` [PATCH printk v1 00/18] threaded/atomic console support Daniel Thompson 2 siblings, 2 replies; 12+ messages in thread From: John Ogness @ 2023-03-02 19:58 UTC (permalink / raw) To: Petr Mladek Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Jason Wessel, Daniel Thompson, Douglas Anderson, Aaron Tomlin, Luis Chamberlain, kgdb-bugreport, Greg Kroah-Hartman, linux-fsdevel, Andrew Morton, Guilherme G. Piccoli, David Gow, Tiezhu Yang, Daniel Vetter, tangmeng, Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes, rcu Implement the necessary callbacks to allow the 8250 console driver to perform as a non-BKL console. Remove the implementation for the legacy console callback (write) and add implementations for the non-BKL consoles (write_atomic, write_thread, port_lock) and add CON_NO_BKL to the initial flags. This is an all-in-one commit meant only for testing the new printk non-BKL infrastructure. It is not meant to be included mainline in this form. In particular, it includes mainline driver fixes that need to be submitted individually. Although non-BKL consoles can coexist with legacy consoles, you will only receive all the benefits of the non-BKL consoles, if this console driver is the only console. That means no netconsole, no tty1, no earlyprintk, no earlycon. Just the uart8250. For example: console=ttyS0,115200 Signed-off-by: John Ogness <john.ogness@linutronix.de> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h index 287153d32536..d8da34bb9ae3 100644 --- a/drivers/tty/serial/8250/8250.h +++ b/drivers/tty/serial/8250/8250.h @@ -177,12 +177,154 @@ static inline void serial_dl_write(struct uart_8250_port *up, int value) up->dl_write(up, value); } +static inline bool serial8250_is_console(struct uart_port *port) +{ + return uart_console(port) && !hlist_unhashed_lockless(&port->cons->node); +} + +static inline void serial8250_init_wctxt(struct cons_write_context *wctxt, + struct console *cons) +{ + struct cons_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt); + + memset(wctxt, 0, sizeof(*wctxt)); + ctxt->console = cons; + ctxt->prio = CONS_PRIO_NORMAL; + /* Both require the port lock, so they cannot clobber each other. */ + ctxt->thread = 1; +} + +static inline void serial8250_console_acquire(struct cons_write_context *wctxt, + struct console *cons) +{ + serial8250_init_wctxt(wctxt, cons); + while (!console_try_acquire(wctxt)) { + cpu_relax(); + serial8250_init_wctxt(wctxt, cons); + } +} + +static inline void serial8250_enter_unsafe(struct uart_8250_port *up) +{ + struct uart_port *port = &up->port; + + lockdep_assert_held_once(&port->lock); + + for (;;) { + up->cookie = console_srcu_read_lock(); + + serial8250_console_acquire(&up->wctxt, port->cons); + + if (console_enter_unsafe(&up->wctxt)) + break; + + console_srcu_read_unlock(up->cookie); + cpu_relax(); + } +} + +static inline void serial8250_exit_unsafe(struct uart_8250_port *up) +{ + struct uart_port *port = &up->port; + + lockdep_assert_held_once(&port->lock); + + /* + * FIXME: The 8250 driver does not support hostile takeovers + * in the unsafe section. + */ + if (!WARN_ON_ONCE(!console_exit_unsafe(&up->wctxt))) + WARN_ON_ONCE(!console_release(&up->wctxt)); + + console_srcu_read_unlock(up->cookie); +} + +static inline int serial8250_in_IER(struct uart_8250_port *up) +{ + struct uart_port *port = &up->port; + bool is_console; + int ier; + + is_console = serial8250_is_console(port); + + if (is_console) + serial8250_enter_unsafe(up); + + ier = serial_in(up, UART_IER); + + if (is_console) + serial8250_exit_unsafe(up); + + return ier; +} + +static inline bool __serial8250_set_IER(struct uart_8250_port *up, + struct cons_write_context *wctxt, + int ier) +{ + if (wctxt && !console_can_proceed(wctxt)) + return false; + serial_out(up, UART_IER, ier); + return true; +} + +static inline void serial8250_set_IER(struct uart_8250_port *up, int ier) +{ + struct uart_port *port = &up->port; + bool is_console; + + is_console = serial8250_is_console(port); + + if (is_console) { + serial8250_enter_unsafe(up); + __serial8250_set_IER(up, &up->wctxt, ier); + serial8250_exit_unsafe(up); + } else { + __serial8250_set_IER(up, NULL, ier); + } +} + +static inline bool __serial8250_clear_IER(struct uart_8250_port *up, + struct cons_write_context *wctxt, + int *prior) +{ + unsigned int clearval = 0; + + if (up->capabilities & UART_CAP_UUE) + clearval = UART_IER_UUE; + + *prior = serial_in(up, UART_IER); + if (wctxt && !console_can_proceed(wctxt)) + return false; + serial_out(up, UART_IER, clearval); + return true; +} + +static inline int serial8250_clear_IER(struct uart_8250_port *up) +{ + struct uart_port *port = &up->port; + bool is_console; + int prior; + + is_console = serial8250_is_console(port); + + if (is_console) { + serial8250_enter_unsafe(up); + __serial8250_clear_IER(up, &up->wctxt, &prior); + serial8250_exit_unsafe(up); + } else { + __serial8250_clear_IER(up, NULL, &prior); + } + + return prior; +} + static inline bool serial8250_set_THRI(struct uart_8250_port *up) { if (up->ier & UART_IER_THRI) return false; up->ier |= UART_IER_THRI; - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); return true; } @@ -191,7 +333,7 @@ static inline bool serial8250_clear_THRI(struct uart_8250_port *up) if (!(up->ier & UART_IER_THRI)) return false; up->ier &= ~UART_IER_THRI; - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); return true; } diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c index 9d2a7856784f..7cc6b527c088 100644 --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c @@ -278,7 +278,7 @@ static void __aspeed_vuart_set_throttle(struct uart_8250_port *up, up->ier &= ~irqs; if (!throttle) up->ier |= irqs; - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); } static void aspeed_vuart_set_throttle(struct uart_port *port, bool throttle) { diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c index ed5a94747692..adb1a3247807 100644 --- a/drivers/tty/serial/8250/8250_bcm7271.c +++ b/drivers/tty/serial/8250/8250_bcm7271.c @@ -606,8 +606,10 @@ static int brcmuart_startup(struct uart_port *port) * Disable the Receive Data Interrupt because the DMA engine * will handle this. */ + spin_lock_irq(&port->lock); up->ier &= ~UART_IER_RDI; - serial_port_out(port, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); + spin_unlock_irq(&port->lock); priv->tx_running = false; priv->dma.rx_dma = NULL; @@ -787,6 +789,12 @@ static int brcmuart_handle_irq(struct uart_port *p) spin_lock_irqsave(&p->lock, flags); status = serial_port_in(p, UART_LSR); if ((status & UART_LSR_DR) == 0) { + bool is_console; + + is_console = serial8250_is_console(port); + + if (is_console) + serial8250_enter_unsafe(p); ier = serial_port_in(p, UART_IER); /* @@ -807,6 +815,9 @@ static int brcmuart_handle_irq(struct uart_port *p) serial_port_in(p, UART_RX); } + if (is_console) + serial8250_exit_unsafe(p); + handled = 1; } spin_unlock_irqrestore(&p->lock, flags); @@ -844,12 +855,22 @@ static enum hrtimer_restart brcmuart_hrtimer_func(struct hrtimer *t) /* re-enable receive unless upper layer has disabled it */ if ((up->ier & (UART_IER_RLSI | UART_IER_RDI)) == (UART_IER_RLSI | UART_IER_RDI)) { + bool is_console; + + is_console = serial8250_is_console(port); + + if (is_console) + serial8250_enter_unsafe(p); + status = serial_port_in(p, UART_IER); status |= (UART_IER_RLSI | UART_IER_RDI); serial_port_out(p, UART_IER, status); status = serial_port_in(p, UART_MCR); status |= UART_MCR_RTS; serial_port_out(p, UART_MCR, status); + + if (is_console) + serial8250_exit_unsafe(p); } spin_unlock_irqrestore(&p->lock, flags); return HRTIMER_NORESTART; diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index ab63c308be0a..688ecfc6e1d5 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -256,6 +256,7 @@ static void serial8250_timeout(struct timer_list *t) static void serial8250_backup_timeout(struct timer_list *t) { struct uart_8250_port *up = from_timer(up, t, timer); + struct uart_port *port = &up->port; unsigned int iir, ier = 0, lsr; unsigned long flags; @@ -266,8 +267,18 @@ static void serial8250_backup_timeout(struct timer_list *t) * based handler. */ if (up->port.irq) { + bool is_console; + + is_console = serial8250_is_console(port); + + if (is_console) + serial8250_enter_unsafe(up); + ier = serial_in(up, UART_IER); serial_out(up, UART_IER, 0); + + if (is_console) + serial8250_exit_unsafe(up); } iir = serial_in(up, UART_IIR); @@ -290,7 +301,7 @@ static void serial8250_backup_timeout(struct timer_list *t) serial8250_tx_chars(up); if (up->port.irq) - serial_out(up, UART_IER, ier); + serial8250_set_IER(up, ier); spin_unlock_irqrestore(&up->port.lock, flags); @@ -576,12 +587,30 @@ serial8250_register_ports(struct uart_driver *drv, struct device *dev) #ifdef CONFIG_SERIAL_8250_CONSOLE -static void univ8250_console_write(struct console *co, const char *s, - unsigned int count) +static void univ8250_console_port_lock(struct console *con, bool do_lock, unsigned long *flags) +{ + struct uart_8250_port *up = &serial8250_ports[con->index]; + + if (do_lock) + spin_lock_irqsave(&up->port.lock, *flags); + else + spin_unlock_irqrestore(&up->port.lock, *flags); +} + +static bool univ8250_console_write_atomic(struct console *co, + struct cons_write_context *wctxt) +{ + struct uart_8250_port *up = &serial8250_ports[co->index]; + + return serial8250_console_write_atomic(up, wctxt); +} + +static bool univ8250_console_write_thread(struct console *co, + struct cons_write_context *wctxt) { struct uart_8250_port *up = &serial8250_ports[co->index]; - serial8250_console_write(up, s, count); + return serial8250_console_write_thread(up, wctxt); } static int univ8250_console_setup(struct console *co, char *options) @@ -669,12 +698,14 @@ static int univ8250_console_match(struct console *co, char *name, int idx, static struct console univ8250_console = { .name = "ttyS", - .write = univ8250_console_write, + .write_atomic = univ8250_console_write_atomic, + .write_thread = univ8250_console_write_thread, + .port_lock = univ8250_console_port_lock, .device = uart_console_device, .setup = univ8250_console_setup, .exit = univ8250_console_exit, .match = univ8250_console_match, - .flags = CON_PRINTBUFFER | CON_ANYTIME, + .flags = CON_PRINTBUFFER | CON_ANYTIME | CON_NO_BKL, .index = -1, .data = &serial8250_reg, }; @@ -962,7 +993,7 @@ static void serial_8250_overrun_backoff_work(struct work_struct *work) spin_lock_irqsave(&port->lock, flags); up->ier |= UART_IER_RLSI | UART_IER_RDI; up->port.read_status_mask |= UART_LSR_DR; - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); spin_unlock_irqrestore(&port->lock, flags); } diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c index 64770c62bbec..ccb70b20b1f4 100644 --- a/drivers/tty/serial/8250/8250_exar.c +++ b/drivers/tty/serial/8250/8250_exar.c @@ -185,6 +185,10 @@ static void xr17v35x_set_divisor(struct uart_port *p, unsigned int baud, static int xr17v35x_startup(struct uart_port *port) { + struct uart_8250_port *up = up_to_u8250p(port); + + spin_lock_irq(&port->lock); + /* * First enable access to IER [7:5], ISR [5:4], FCR [5:4], * MCR [7:5] and MSR [7:0] @@ -195,7 +199,9 @@ static int xr17v35x_startup(struct uart_port *port) * Make sure all interrups are masked until initialization is * complete and the FIFOs are cleared */ - serial_port_out(port, UART_IER, 0); + serial8250_set_IER(up, 0); + + spin_unlock_irq(&port->lock); return serial8250_do_startup(port); } diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c index 8aad15622a2e..74bb85b705e7 100644 --- a/drivers/tty/serial/8250/8250_fsl.c +++ b/drivers/tty/serial/8250/8250_fsl.c @@ -58,7 +58,8 @@ int fsl8250_handle_irq(struct uart_port *port) if ((orig_lsr & UART_LSR_OE) && (up->overrun_backoff_time_ms > 0)) { unsigned long delay; - up->ier = port->serial_in(port, UART_IER); + up->ier = serial8250_in_IER(up); + if (up->ier & (UART_IER_RLSI | UART_IER_RDI)) { port->ops->stop_rx(port); } else { diff --git a/drivers/tty/serial/8250/8250_ingenic.c b/drivers/tty/serial/8250/8250_ingenic.c index 617b8ce60d6b..548904c3d11b 100644 --- a/drivers/tty/serial/8250/8250_ingenic.c +++ b/drivers/tty/serial/8250/8250_ingenic.c @@ -171,6 +171,7 @@ OF_EARLYCON_DECLARE(x1000_uart, "ingenic,x1000-uart", static void ingenic_uart_serial_out(struct uart_port *p, int offset, int value) { + struct uart_8250_port *up = up_to_u8250p(p); int ier; switch (offset) { @@ -192,7 +193,7 @@ static void ingenic_uart_serial_out(struct uart_port *p, int offset, int value) * If we have enabled modem status IRQs we should enable * modem mode. */ - ier = p->serial_in(p, UART_IER); + ier = serial8250_in_IER(up); if (ier & UART_IER_MSI) value |= UART_MCR_MDCE | UART_MCR_FCM; diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c index fb1d5ec0940e..bf7ab55c8923 100644 --- a/drivers/tty/serial/8250/8250_mtk.c +++ b/drivers/tty/serial/8250/8250_mtk.c @@ -222,12 +222,38 @@ static void mtk8250_shutdown(struct uart_port *port) static void mtk8250_disable_intrs(struct uart_8250_port *up, int mask) { - serial_out(up, UART_IER, serial_in(up, UART_IER) & (~mask)); + struct uart_port *port = &up->port; + bool is_console; + int ier; + + is_console = serial8250_is_console(port); + + if (is_console) + serial8250_enter_unsafe(up); + + ier = serial_in(up, UART_IER); + serial_out(up, UART_IER, ier & (~mask)); + + if (is_console) + serial8250_exit_unsafe(up); } static void mtk8250_enable_intrs(struct uart_8250_port *up, int mask) { - serial_out(up, UART_IER, serial_in(up, UART_IER) | mask); + struct uart_port *port = &up->port; + bool is_console; + int ier; + + is_console = serial8250_is_console(port); + + if (is_console) + serial8250_enter_unsafe(up); + + ier = serial_in(up, UART_IER); + serial_out(up, UART_IER, ier | mask); + + if (is_console) + serial8250_exit_unsafe(up); } static void mtk8250_set_flow_ctrl(struct uart_8250_port *up, int mode) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 734f092ef839..bfa50a26349d 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -334,8 +334,7 @@ static void omap8250_restore_regs(struct uart_8250_port *up) /* drop TCR + TLR access, we setup XON/XOFF later */ serial8250_out_MCR(up, mcr); - - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); serial_dl_write(up, priv->quot); @@ -523,16 +522,21 @@ static void omap_8250_pm(struct uart_port *port, unsigned int state, u8 efr; pm_runtime_get_sync(port->dev); + + spin_lock_irq(&port->lock); + serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); efr = serial_in(up, UART_EFR); serial_out(up, UART_EFR, efr | UART_EFR_ECB); serial_out(up, UART_LCR, 0); - serial_out(up, UART_IER, (state != 0) ? UART_IERX_SLEEP : 0); + serial8250_set_IER(up, (state != 0) ? UART_IERX_SLEEP : 0); serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); serial_out(up, UART_EFR, efr); serial_out(up, UART_LCR, 0); + spin_unlock_irq(&port->lock); + pm_runtime_mark_last_busy(port->dev); pm_runtime_put_autosuspend(port->dev); } @@ -649,7 +653,8 @@ static irqreturn_t omap8250_irq(int irq, void *dev_id) if ((lsr & UART_LSR_OE) && up->overrun_backoff_time_ms > 0) { unsigned long delay; - up->ier = port->serial_in(port, UART_IER); + spin_lock(&port->lock); + up->ier = serial8250_in_IER(up); if (up->ier & (UART_IER_RLSI | UART_IER_RDI)) { port->ops->stop_rx(port); } else { @@ -658,6 +663,7 @@ static irqreturn_t omap8250_irq(int irq, void *dev_id) */ cancel_delayed_work(&up->overrun_backoff); } + spin_unlock(&port->lock); delay = msecs_to_jiffies(up->overrun_backoff_time_ms); schedule_delayed_work(&up->overrun_backoff, delay); @@ -707,8 +713,10 @@ static int omap_8250_startup(struct uart_port *port) if (ret < 0) goto err; + spin_lock_irq(&port->lock); up->ier = UART_IER_RLSI | UART_IER_RDI; - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); + spin_unlock_irq(&port->lock); #ifdef CONFIG_PM up->capabilities |= UART_CAP_RPM; @@ -748,8 +756,10 @@ static void omap_8250_shutdown(struct uart_port *port) if (priv->habit & UART_HAS_EFR2) serial_out(up, UART_OMAP_EFR2, 0x0); + spin_lock_irq(&port->lock); up->ier = 0; - serial_out(up, UART_IER, 0); + serial8250_set_IER(up, 0); + spin_unlock_irq(&port->lock); if (up->dma) serial8250_release_dma(up); @@ -797,7 +807,7 @@ static void omap_8250_unthrottle(struct uart_port *port) up->dma->rx_dma(up); up->ier |= UART_IER_RLSI | UART_IER_RDI; port->read_status_mask |= UART_LSR_DR; - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); spin_unlock_irqrestore(&port->lock, flags); pm_runtime_mark_last_busy(port->dev); @@ -956,7 +966,7 @@ static void __dma_rx_complete(void *param) __dma_rx_do_complete(p); if (!priv->throttled) { p->ier |= UART_IER_RLSI | UART_IER_RDI; - serial_out(p, UART_IER, p->ier); + serial8250_set_IER(p, p->ier); if (!(priv->habit & UART_HAS_EFR2)) omap_8250_rx_dma(p); } @@ -1013,7 +1023,7 @@ static int omap_8250_rx_dma(struct uart_8250_port *p) * callback to run. */ p->ier &= ~(UART_IER_RLSI | UART_IER_RDI); - serial_out(p, UART_IER, p->ier); + serial8250_set_IER(p, p->ier); } goto out; } @@ -1226,12 +1236,12 @@ static void am654_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir, * periodic timeouts, re-enable interrupts. */ up->ier &= ~(UART_IER_RLSI | UART_IER_RDI); - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); omap_8250_rx_dma_flush(up); serial_in(up, UART_IIR); serial_out(up, UART_OMAP_EFR2, 0x0); up->ier |= UART_IER_RLSI | UART_IER_RDI; - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); } } @@ -1717,12 +1727,16 @@ static int omap8250_runtime_resume(struct device *dev) up = serial8250_get_port(priv->line); + spin_lock_irq(&up->port.lock); + if (omap8250_lost_context(up)) omap8250_restore_regs(up); if (up->dma && up->dma->rxchan && !(priv->habit & UART_HAS_EFR2)) omap_8250_rx_dma(up); + spin_unlock_irq(&up->port.lock); + priv->latency = priv->calc_latency; schedule_work(&priv->qos_work); return 0; diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index fa43df05342b..f1976d9a8a38 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -744,6 +744,7 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) serial8250_rpm_get(p); if (p->capabilities & UART_CAP_SLEEP) { + spin_lock_irq(&p->port.lock); if (p->capabilities & UART_CAP_EFR) { lcr = serial_in(p, UART_LCR); efr = serial_in(p, UART_EFR); @@ -751,25 +752,18 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) serial_out(p, UART_EFR, UART_EFR_ECB); serial_out(p, UART_LCR, 0); } - serial_out(p, UART_IER, sleep ? UART_IERX_SLEEP : 0); + serial8250_set_IER(p, sleep ? UART_IERX_SLEEP : 0); if (p->capabilities & UART_CAP_EFR) { serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B); serial_out(p, UART_EFR, efr); serial_out(p, UART_LCR, lcr); } + spin_unlock_irq(&p->port.lock); } serial8250_rpm_put(p); } -static void serial8250_clear_IER(struct uart_8250_port *up) -{ - if (up->capabilities & UART_CAP_UUE) - serial_out(up, UART_IER, UART_IER_UUE); - else - serial_out(up, UART_IER, 0); -} - #ifdef CONFIG_SERIAL_8250_RSA /* * Attempts to turn on the RSA FIFO. Returns zero on failure. @@ -1033,8 +1027,10 @@ static int broken_efr(struct uart_8250_port *up) */ static void autoconfig_16550a(struct uart_8250_port *up) { + struct uart_port *port = &up->port; unsigned char status1, status2; unsigned int iersave; + bool is_console; up->port.type = PORT_16550A; up->capabilities |= UART_CAP_FIFO; @@ -1150,6 +1146,11 @@ static void autoconfig_16550a(struct uart_8250_port *up) return; } + is_console = serial8250_is_console(port); + + if (is_console) + serial8250_enter_unsafe(up); + /* * Try writing and reading the UART_IER_UUE bit (b6). * If it works, this is probably one of the Xscale platform's @@ -1185,6 +1186,9 @@ static void autoconfig_16550a(struct uart_8250_port *up) } serial_out(up, UART_IER, iersave); + if (is_console) + serial8250_exit_unsafe(up); + /* * We distinguish between 16550A and U6 16550A by counting * how many bytes are in the FIFO. @@ -1226,6 +1230,13 @@ static void autoconfig(struct uart_8250_port *up) up->bugs = 0; if (!(port->flags & UPF_BUGGY_UART)) { + bool is_console; + + is_console = serial8250_is_console(port); + + if (is_console) + serial8250_enter_unsafe(up); + /* * Do a simple existence test first; if we fail this, * there's no point trying anything else. @@ -1255,6 +1266,10 @@ static void autoconfig(struct uart_8250_port *up) #endif scratch3 = serial_in(up, UART_IER) & UART_IER_ALL_INTR; serial_out(up, UART_IER, scratch); + + if (is_console) + serial8250_exit_unsafe(up); + if (scratch2 != 0 || scratch3 != UART_IER_ALL_INTR) { /* * We failed; there's nothing here @@ -1376,6 +1391,7 @@ static void autoconfig_irq(struct uart_8250_port *up) unsigned char save_ICP = 0; unsigned int ICP = 0; unsigned long irqs; + bool is_console; int irq; if (port->flags & UPF_FOURPORT) { @@ -1385,8 +1401,12 @@ static void autoconfig_irq(struct uart_8250_port *up) inb_p(ICP); } - if (uart_console(port)) + is_console = serial8250_is_console(port); + + if (is_console) { console_lock(); + serial8250_enter_unsafe(up); + } /* forget possible initially masked and pending IRQ */ probe_irq_off(probe_irq_on()); @@ -1418,8 +1438,10 @@ static void autoconfig_irq(struct uart_8250_port *up) if (port->flags & UPF_FOURPORT) outb_p(save_ICP, ICP); - if (uart_console(port)) + if (is_console) { + serial8250_exit_unsafe(up); console_unlock(); + } port->irq = (irq > 0) ? irq : 0; } @@ -1432,7 +1454,7 @@ static void serial8250_stop_rx(struct uart_port *port) up->ier &= ~(UART_IER_RLSI | UART_IER_RDI); up->port.read_status_mask &= ~UART_LSR_DR; - serial_port_out(port, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); serial8250_rpm_put(up); } @@ -1462,7 +1484,7 @@ void serial8250_em485_stop_tx(struct uart_8250_port *p) serial8250_clear_and_reinit_fifos(p); p->ier |= UART_IER_RLSI | UART_IER_RDI; - serial_port_out(&p->port, UART_IER, p->ier); + serial8250_set_IER(p, p->ier); } } EXPORT_SYMBOL_GPL(serial8250_em485_stop_tx); @@ -1709,7 +1731,7 @@ static void serial8250_disable_ms(struct uart_port *port) mctrl_gpio_disable_ms(up->gpios); up->ier &= ~UART_IER_MSI; - serial_port_out(port, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); } static void serial8250_enable_ms(struct uart_port *port) @@ -1725,7 +1747,7 @@ static void serial8250_enable_ms(struct uart_port *port) up->ier |= UART_IER_MSI; serial8250_rpm_get(up); - serial_port_out(port, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); serial8250_rpm_put(up); } @@ -2160,9 +2182,10 @@ static void serial8250_put_poll_char(struct uart_port *port, serial8250_rpm_get(up); /* * First save the IER then disable the interrupts + * + * Best-effort IER access because other CPUs are quiesced. */ - ier = serial_port_in(port, UART_IER); - serial8250_clear_IER(up); + __serial8250_clear_IER(up, NULL, &ier); wait_for_xmitr(up, UART_LSR_BOTH_EMPTY); /* @@ -2175,7 +2198,7 @@ static void serial8250_put_poll_char(struct uart_port *port, * and restore the IER */ wait_for_xmitr(up, UART_LSR_BOTH_EMPTY); - serial_port_out(port, UART_IER, ier); + __serial8250_set_IER(up, NULL, ier); serial8250_rpm_put(up); } @@ -2186,6 +2209,7 @@ int serial8250_do_startup(struct uart_port *port) struct uart_8250_port *up = up_to_u8250p(port); unsigned long flags; unsigned char iir; + bool is_console; int retval; u16 lsr; @@ -2203,21 +2227,25 @@ int serial8250_do_startup(struct uart_port *port) serial8250_rpm_get(up); if (port->type == PORT_16C950) { /* Wake up and initialize UART */ + spin_lock_irqsave(&port->lock, flags); up->acr = 0; serial_port_out(port, UART_LCR, UART_LCR_CONF_MODE_B); serial_port_out(port, UART_EFR, UART_EFR_ECB); - serial_port_out(port, UART_IER, 0); + serial8250_set_IER(up, 0); serial_port_out(port, UART_LCR, 0); serial_icr_write(up, UART_CSR, 0); /* Reset the UART */ serial_port_out(port, UART_LCR, UART_LCR_CONF_MODE_B); serial_port_out(port, UART_EFR, UART_EFR_ECB); serial_port_out(port, UART_LCR, 0); + spin_unlock_irqrestore(&port->lock, flags); } if (port->type == PORT_DA830) { /* Reset the port */ - serial_port_out(port, UART_IER, 0); + spin_lock_irqsave(&port->lock, flags); + serial8250_set_IER(up, 0); serial_port_out(port, UART_DA830_PWREMU_MGMT, 0); + spin_unlock_irqrestore(&port->lock, flags); mdelay(10); /* Enable Tx, Rx and free run mode */ @@ -2315,6 +2343,8 @@ int serial8250_do_startup(struct uart_port *port) if (retval) goto out; + is_console = serial8250_is_console(port); + if (port->irq && !(up->port.flags & UPF_NO_THRE_TEST)) { unsigned char iir1; @@ -2331,6 +2361,9 @@ int serial8250_do_startup(struct uart_port *port) */ spin_lock_irqsave(&port->lock, flags); + if (is_console) + serial8250_enter_unsafe(up); + wait_for_xmitr(up, UART_LSR_THRE); serial_port_out_sync(port, UART_IER, UART_IER_THRI); udelay(1); /* allow THRE to set */ @@ -2341,6 +2374,9 @@ int serial8250_do_startup(struct uart_port *port) iir = serial_port_in(port, UART_IIR); serial_port_out(port, UART_IER, 0); + if (is_console) + serial8250_exit_unsafe(up); + spin_unlock_irqrestore(&port->lock, flags); if (port->irqflags & IRQF_SHARED) @@ -2395,10 +2431,14 @@ int serial8250_do_startup(struct uart_port *port) * Do a quick test to see if we receive an interrupt when we enable * the TX irq. */ + if (is_console) + serial8250_enter_unsafe(up); serial_port_out(port, UART_IER, UART_IER_THRI); lsr = serial_port_in(port, UART_LSR); iir = serial_port_in(port, UART_IIR); serial_port_out(port, UART_IER, 0); + if (is_console) + serial8250_exit_unsafe(up); if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT) { if (!(up->bugs & UART_BUG_TXEN)) { @@ -2430,7 +2470,7 @@ int serial8250_do_startup(struct uart_port *port) if (up->dma) { const char *msg = NULL; - if (uart_console(port)) + if (is_console) msg = "forbid DMA for kernel console"; else if (serial8250_request_dma(up)) msg = "failed to request DMA"; @@ -2481,7 +2521,7 @@ void serial8250_do_shutdown(struct uart_port *port) */ spin_lock_irqsave(&port->lock, flags); up->ier = 0; - serial_port_out(port, UART_IER, 0); + serial8250_set_IER(up, 0); spin_unlock_irqrestore(&port->lock, flags); synchronize_irq(port->irq); @@ -2847,7 +2887,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios, if (up->capabilities & UART_CAP_RTOIE) up->ier |= UART_IER_RTOIE; - serial_port_out(port, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); if (up->capabilities & UART_CAP_EFR) { unsigned char efr = 0; @@ -3312,12 +3352,21 @@ EXPORT_SYMBOL_GPL(serial8250_set_defaults); #ifdef CONFIG_SERIAL_8250_CONSOLE -static void serial8250_console_putchar(struct uart_port *port, unsigned char ch) +static bool serial8250_console_putchar(struct uart_port *port, unsigned char ch, + struct cons_write_context *wctxt) { struct uart_8250_port *up = up_to_u8250p(port); wait_for_xmitr(up, UART_LSR_THRE); + if (!console_can_proceed(wctxt)) + return false; serial_port_out(port, UART_TX, ch); + if (ch == '\n') + up->console_newline_needed = false; + else + up->console_newline_needed = true; + + return true; } /* @@ -3346,33 +3395,134 @@ static void serial8250_console_restore(struct uart_8250_port *up) serial8250_out_MCR(up, up->mcr | UART_MCR_DTR | UART_MCR_RTS); } +static bool __serial8250_console_write(struct uart_port *port, struct cons_write_context *wctxt, + const char *s, unsigned int count, + bool (*putchar)(struct uart_port *, unsigned char, struct cons_write_context *)) +{ + bool finished = false; + unsigned int i; + + for (i = 0; i < count; i++, s++) { + if (*s == '\n') { + if (!putchar(port, '\r', wctxt)) + goto out; + } + if (!putchar(port, *s, wctxt)) + goto out; + } + finished = true; +out: + return finished; +} + +static bool serial8250_console_write(struct uart_port *port, struct cons_write_context *wctxt, + const char *s, unsigned int count, + bool (*putchar)(struct uart_port *, unsigned char, struct cons_write_context *)) +{ + return __serial8250_console_write(port, wctxt, s, count, putchar); +} + +static bool atomic_print_line(struct uart_8250_port *up, + struct cons_write_context *wctxt) +{ + struct uart_port *port = &up->port; + char buf[4]; + + if (up->console_newline_needed && + !__serial8250_console_write(port, wctxt, "\n", 1, serial8250_console_putchar)) { + return false; + } + + sprintf(buf, "A%d", raw_smp_processor_id()); + if (!__serial8250_console_write(port, wctxt, buf, strlen(buf), serial8250_console_putchar)) + return false; + + return __serial8250_console_write(port, wctxt, wctxt->outbuf, wctxt->len, + serial8250_console_putchar); +} + +static void atomic_console_reacquire(struct cons_write_context *wctxt, + struct cons_write_context *wctxt_init) +{ + memcpy(wctxt, wctxt_init, sizeof(*wctxt)); + while (!console_try_acquire(wctxt)) { + cpu_relax(); + memcpy(wctxt, wctxt_init, sizeof(*wctxt)); + } +} + /* - * Print a string to the serial port using the device FIFO - * - * It sends fifosize bytes and then waits for the fifo - * to get empty. + * It should be possible to support a hostile takeover in an unsafe + * section if it is write_atomic() that is being taken over. But where + * to put this policy? */ -static void serial8250_console_fifo_write(struct uart_8250_port *up, - const char *s, unsigned int count) +bool serial8250_console_write_atomic(struct uart_8250_port *up, + struct cons_write_context *wctxt) { - int i; - const char *end = s + count; - unsigned int fifosize = up->tx_loadsz; - bool cr_sent = false; - - while (s != end) { - wait_for_lsr(up, UART_LSR_THRE); - - for (i = 0; i < fifosize && s != end; ++i) { - if (*s == '\n' && !cr_sent) { - serial_out(up, UART_TX, '\r'); - cr_sent = true; - } else { - serial_out(up, UART_TX, *s++); - cr_sent = false; - } + struct cons_write_context wctxt_init = {}; + struct cons_context *ctxt_init = &ACCESS_PRIVATE(&wctxt_init, ctxt); + struct cons_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt); + bool can_print = true; + unsigned int ier; + + /* With write_atomic, another context may hold the port->lock. */ + + ctxt_init->console = ctxt->console; + ctxt_init->prio = ctxt->prio; + ctxt_init->thread = ctxt->thread; + + touch_nmi_watchdog(); + + /* + * Enter unsafe in order to disable interrupts. If the console is + * lost before the interrupts are disabled, bail out because another + * context took over the printing. If the console is lost after the + * interrutps are disabled, the console must be reacquired in order + * to re-enable the interrupts. However in that case no printing is + * allowed because another context took over the printing. + */ + + if (!console_enter_unsafe(wctxt)) + return false; + + if (!__serial8250_clear_IER(up, wctxt, &ier)) + return false; + + if (console_exit_unsafe(wctxt)) { + can_print = atomic_print_line(up, wctxt); + if (!can_print) + atomic_console_reacquire(wctxt, &wctxt_init); + + if (can_print) { + can_print = console_can_proceed(wctxt); + if (can_print) + wait_for_xmitr(up, UART_LSR_BOTH_EMPTY); + else + atomic_console_reacquire(wctxt, &wctxt_init); + } + } else { + atomic_console_reacquire(wctxt, &wctxt_init); + } + + /* + * Enter unsafe in order to enable interrupts. If the console is + * lost before the interrupts are enabled, the console must be + * reacquired in order to re-enable the interrupts. + */ + + for (;;) { + if (console_enter_unsafe(wctxt) && + __serial8250_set_IER(up, wctxt, ier)) { + break; } + + /* HW-IRQs still disabled. Reacquire to enable them. */ + atomic_console_reacquire(wctxt, &wctxt_init); } + + console_exit_unsafe(wctxt); + + return can_print; } /* @@ -3384,64 +3534,54 @@ static void serial8250_console_fifo_write(struct uart_8250_port *up, * Doing runtime PM is really a bad idea for the kernel console. * Thus, we assume the function is called when device is powered up. */ -void serial8250_console_write(struct uart_8250_port *up, const char *s, - unsigned int count) +bool serial8250_console_write_thread(struct uart_8250_port *up, + struct cons_write_context *wctxt) { struct uart_8250_em485 *em485 = up->em485; struct uart_port *port = &up->port; - unsigned long flags; - unsigned int ier, use_fifo; - int locked = 1; - - touch_nmi_watchdog(); - - if (oops_in_progress) - locked = spin_trylock_irqsave(&port->lock, flags); - else - spin_lock_irqsave(&port->lock, flags); + unsigned int count = wctxt->len; + const char *s = wctxt->outbuf; + bool finished = false; + unsigned int ier; + char buf[4]; /* * First save the IER then disable the interrupts */ - ier = serial_port_in(port, UART_IER); - serial8250_clear_IER(up); + if (!console_enter_unsafe(wctxt) || + !__serial8250_clear_IER(up, wctxt, &ier)) { + goto out; + } + if (!console_exit_unsafe(wctxt)) + goto out; /* check scratch reg to see if port powered off during system sleep */ if (up->canary && (up->canary != serial_port_in(port, UART_SCR))) { + if (!console_enter_unsafe(wctxt)) + goto out; serial8250_console_restore(up); + if (!console_exit_unsafe(wctxt)) + goto out; up->canary = 0; } if (em485) { - if (em485->tx_stopped) + if (em485->tx_stopped) { + if (!console_enter_unsafe(wctxt)) + goto out; up->rs485_start_tx(up); - mdelay(port->rs485.delay_rts_before_send); + if (!console_exit_unsafe(wctxt)) + goto out; + } + mdelay(port->rs485.delay_rts_before_send); /* WTF?! Seriously?! */ } - use_fifo = (up->capabilities & UART_CAP_FIFO) && - /* - * BCM283x requires to check the fifo - * after each byte. - */ - !(up->capabilities & UART_CAP_MINI) && - /* - * tx_loadsz contains the transmit fifo size - */ - up->tx_loadsz > 1 && - (up->fcr & UART_FCR_ENABLE_FIFO) && - port->state && - test_bit(TTY_PORT_INITIALIZED, &port->state->port.iflags) && - /* - * After we put a data in the fifo, the controller will send - * it regardless of the CTS state. Therefore, only use fifo - * if we don't use control flow. - */ - !(up->port.flags & UPF_CONS_FLOW); + sprintf(buf, "T%d", raw_smp_processor_id()); + if (serial8250_console_write(port, wctxt, buf, strlen(buf), serial8250_console_putchar)) + finished = serial8250_console_write(port, wctxt, s, count, serial8250_console_putchar); - if (likely(use_fifo)) - serial8250_console_fifo_write(up, s, count); - else - uart_console_write(port, s, count, serial8250_console_putchar); + if (!finished) + goto out; /* * Finally, wait for transmitter to become empty @@ -3450,12 +3590,20 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s, wait_for_xmitr(up, UART_LSR_BOTH_EMPTY); if (em485) { - mdelay(port->rs485.delay_rts_after_send); - if (em485->tx_stopped) + mdelay(port->rs485.delay_rts_after_send); /* WTF?! Seriously?! */ + if (em485->tx_stopped) { + if (!console_enter_unsafe(wctxt)) + goto out; up->rs485_stop_tx(up); + if (!console_exit_unsafe(wctxt)) + goto out; + } } - - serial_port_out(port, UART_IER, ier); + if (!console_enter_unsafe(wctxt)) + goto out; + WARN_ON_ONCE(!__serial8250_set_IER(up, wctxt, ier)); + if (!console_exit_unsafe(wctxt)) + goto out; /* * The receive handling will happen properly because the @@ -3464,11 +3612,15 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s, * call it if we have saved something in the saved flags * while processing with interrupts off. */ - if (up->msr_saved_flags) + if (up->msr_saved_flags) { + if (!console_enter_unsafe(wctxt)) + goto out; serial8250_modem_status(up); - - if (locked) - spin_unlock_irqrestore(&port->lock, flags); + if (!console_exit_unsafe(wctxt)) + goto out; + } +out: + return finished; } static unsigned int probe_baud(struct uart_port *port) @@ -3488,6 +3640,7 @@ static unsigned int probe_baud(struct uart_port *port) int serial8250_console_setup(struct uart_port *port, char *options, bool probe) { + struct uart_8250_port *up = up_to_u8250p(port); int baud = 9600; int bits = 8; int parity = 'n'; @@ -3497,6 +3650,8 @@ int serial8250_console_setup(struct uart_port *port, char *options, bool probe) if (!port->iobase && !port->membase) return -ENODEV; + up->console_newline_needed = false; + if (options) uart_parse_options(options, &baud, &parity, &bits, &flow); else if (probe) diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig index 978dc196c29b..22656e8370ea 100644 --- a/drivers/tty/serial/8250/Kconfig +++ b/drivers/tty/serial/8250/Kconfig @@ -9,6 +9,7 @@ config SERIAL_8250 depends on !S390 select SERIAL_CORE select SERIAL_MCTRL_GPIO if GPIOLIB + select HAVE_ATOMIC_CONSOLE help This selects whether you want to include the driver for the standard serial ports. The standard answer is Y. People who might say N diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 2bd32c8ece39..9901f916dc1a 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -2336,8 +2336,11 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport) * able to Re-start_rx later. */ if (!console_suspend_enabled && uart_console(uport)) { - if (uport->ops->start_rx) + if (uport->ops->start_rx) { + spin_lock_irq(&uport->lock); uport->ops->stop_rx(uport); + spin_unlock_irq(&uport->lock); + } goto unlock; } @@ -2430,8 +2433,11 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport) if (console_suspend_enabled) uart_change_pm(state, UART_PM_STATE_ON); uport->ops->set_termios(uport, &termios, NULL); - if (!console_suspend_enabled && uport->ops->start_rx) + if (!console_suspend_enabled && uport->ops->start_rx) { + spin_lock_irq(&uport->lock); uport->ops->start_rx(uport); + spin_unlock_irq(&uport->lock); + } if (console_suspend_enabled) console_start(uport->cons); } diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h index 19376bee9667..9055a22992ed 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -125,6 +125,8 @@ struct uart_8250_port { #define MSR_SAVE_FLAGS UART_MSR_ANY_DELTA unsigned char msr_saved_flags; + bool console_newline_needed; + struct uart_8250_dma *dma; const struct uart_8250_ops *ops; @@ -139,6 +141,9 @@ struct uart_8250_port { /* Serial port overrun backoff */ struct delayed_work overrun_backoff; u32 overrun_backoff_time_ms; + + struct cons_write_context wctxt; + int cookie; }; static inline struct uart_8250_port *up_to_u8250p(struct uart_port *up) @@ -178,8 +183,10 @@ void serial8250_tx_chars(struct uart_8250_port *up); unsigned int serial8250_modem_status(struct uart_8250_port *up); void serial8250_init_port(struct uart_8250_port *up); void serial8250_set_defaults(struct uart_8250_port *up); -void serial8250_console_write(struct uart_8250_port *up, const char *s, - unsigned int count); +bool serial8250_console_write_atomic(struct uart_8250_port *up, + struct cons_write_context *wctxt); +bool serial8250_console_write_thread(struct uart_8250_port *up, + struct cons_write_context *wctxt); int serial8250_console_setup(struct uart_port *port, char *options, bool probe); int serial8250_console_exit(struct uart_port *port); ^ permalink raw reply related [flat|nested] 12+ messages in thread
* locking API: was: [PATCH printk v1 00/18] serial: 8250: implement non-BKL console 2023-03-02 19:58 ` [PATCH printk v1 00/18] serial: 8250: implement non-BKL console John Ogness @ 2023-03-28 13:33 ` Petr Mladek 2023-03-28 13:57 ` John Ogness 2023-03-28 13:59 ` [PATCH printk v1 00/18] POC: serial: 8250: implement nbcon console John Ogness 1 sibling, 1 reply; 12+ messages in thread From: Petr Mladek @ 2023-03-28 13:33 UTC (permalink / raw) To: John Ogness Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Jason Wessel, Daniel Thompson, Douglas Anderson, Aaron Tomlin, Luis Chamberlain, kgdb-bugreport, Greg Kroah-Hartman, linux-fsdevel, Andrew Morton, Guilherme G. Piccoli, David Gow, Tiezhu Yang, Daniel Vetter, tangmeng, Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes, rcu On Thu 2023-03-02 21:04:50, John Ogness wrote: > Implement the necessary callbacks to allow the 8250 console driver > to perform as a non-BKL console. Remove the implementation for the > legacy console callback (write) and add implementations for the > non-BKL consoles (write_atomic, write_thread, port_lock) and add > CON_NO_BKL to the initial flags. > > This is an all-in-one commit meant only for testing the new printk > non-BKL infrastructure. It is not meant to be included mainline in > this form. In particular, it includes mainline driver fixes that > need to be submitted individually. > > Although non-BKL consoles can coexist with legacy consoles, you > will only receive all the benefits of the non-BKL consoles, if > this console driver is the only console. That means no netconsole, > no tty1, no earlyprintk, no earlycon. Just the uart8250. > > For example: console=ttyS0,115200 > > --- a/drivers/tty/serial/8250/8250_port.c > +++ b/drivers/tty/serial/8250/8250_port.c > +static void atomic_console_reacquire(struct cons_write_context *wctxt, > + struct cons_write_context *wctxt_init) > +{ > + memcpy(wctxt, wctxt_init, sizeof(*wctxt)); > + while (!console_try_acquire(wctxt)) { > + cpu_relax(); > + memcpy(wctxt, wctxt_init, sizeof(*wctxt)); > + } > +} > + > /* > - * Print a string to the serial port using the device FIFO > - * > - * It sends fifosize bytes and then waits for the fifo > - * to get empty. > + * It should be possible to support a hostile takeover in an unsafe > + * section if it is write_atomic() that is being taken over. But where > + * to put this policy? > */ > -static void serial8250_console_fifo_write(struct uart_8250_port *up, > - const char *s, unsigned int count) > +bool serial8250_console_write_atomic(struct uart_8250_port *up, > + struct cons_write_context *wctxt) > { > - int i; > - const char *end = s + count; > - unsigned int fifosize = up->tx_loadsz; > - bool cr_sent = false; > - > - while (s != end) { > - wait_for_lsr(up, UART_LSR_THRE); > - > - for (i = 0; i < fifosize && s != end; ++i) { > - if (*s == '\n' && !cr_sent) { > - serial_out(up, UART_TX, '\r'); > - cr_sent = true; > - } else { > - serial_out(up, UART_TX, *s++); > - cr_sent = false; > - } > + struct cons_write_context wctxt_init = {}; > + struct cons_context *ctxt_init = &ACCESS_PRIVATE(&wctxt_init, ctxt); > + struct cons_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt); > + bool can_print = true; > + unsigned int ier; > + > + /* With write_atomic, another context may hold the port->lock. */ > + > + ctxt_init->console = ctxt->console; > + ctxt_init->prio = ctxt->prio; > + ctxt_init->thread = ctxt->thread; > + > + touch_nmi_watchdog(); > + > + /* > + * Enter unsafe in order to disable interrupts. If the console is > + * lost before the interrupts are disabled, bail out because another > + * context took over the printing. If the console is lost after the > + * interrutps are disabled, the console must be reacquired in order > + * to re-enable the interrupts. However in that case no printing is > + * allowed because another context took over the printing. > + */ > + > + if (!console_enter_unsafe(wctxt)) > + return false; > + > + if (!__serial8250_clear_IER(up, wctxt, &ier)) > + return false; > + > + if (console_exit_unsafe(wctxt)) { > + can_print = atomic_print_line(up, wctxt); > + if (!can_print) > + atomic_console_reacquire(wctxt, &wctxt_init); I am trying to review the 9th patch adding console_can_proceed(), console_enter_unsafe(), console_exit_unsafe() API. And I wanted to see how the struct cons_write_context was actually used. I am confused now. I do not understand the motivation for the extra @wctxt_init copy and atomic_console_reacquire(). Why do we need a copy? And why we need to reacquire it? My feeling is that it is needed only to call console_exit_unsafe(wctxt) later. Or do I miss anything? > + > + if (can_print) { > + can_print = console_can_proceed(wctxt); > + if (can_print) > + wait_for_xmitr(up, UART_LSR_BOTH_EMPTY); > + else > + atomic_console_reacquire(wctxt, &wctxt_init); > + } > + } else { > + atomic_console_reacquire(wctxt, &wctxt_init); > + } > + > + /* > + * Enter unsafe in order to enable interrupts. If the console is > + * lost before the interrupts are enabled, the console must be > + * reacquired in order to re-enable the interrupts. > + */ > + > + for (;;) { > + if (console_enter_unsafe(wctxt) && > + __serial8250_set_IER(up, wctxt, ier)) { > + break; > } > + > + /* HW-IRQs still disabled. Reacquire to enable them. */ > + atomic_console_reacquire(wctxt, &wctxt_init); > } > + > + console_exit_unsafe(wctxt); > + > + return can_print; > } Best Regards, Petr ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: locking API: was: [PATCH printk v1 00/18] serial: 8250: implement non-BKL console 2023-03-28 13:33 ` locking API: was: " Petr Mladek @ 2023-03-28 13:57 ` John Ogness 2023-03-28 15:10 ` Petr Mladek 0 siblings, 1 reply; 12+ messages in thread From: John Ogness @ 2023-03-28 13:57 UTC (permalink / raw) To: Petr Mladek Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Jason Wessel, Daniel Thompson, Douglas Anderson, Aaron Tomlin, Luis Chamberlain, kgdb-bugreport, Greg Kroah-Hartman, linux-fsdevel, Andrew Morton, Guilherme G. Piccoli, David Gow, Tiezhu Yang, Daniel Vetter, tangmeng, Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes, rcu On 2023-03-28, Petr Mladek <pmladek@suse.com> wrote: >> + if (!__serial8250_clear_IER(up, wctxt, &ier)) >> + return false; >> + >> + if (console_exit_unsafe(wctxt)) { >> + can_print = atomic_print_line(up, wctxt); >> + if (!can_print) >> + atomic_console_reacquire(wctxt, &wctxt_init); > > I am trying to review the 9th patch adding console_can_proceed(), > console_enter_unsafe(), console_exit_unsafe() API. And I wanted > to see how the struct cons_write_context was actually used. First off, I need to post the latest version of the 8250-POC patch. It is not officially part of this series and is still going through changes for the PREEMPT_RT tree. I will post the latest version directly after answering this email. > I am confused now. I do not understand the motivation for the extra > @wctxt_init copy and atomic_console_reacquire(). If an atomic context loses ownership while doing certain activities, it may need to re-acquire ownership in order to finish or cleanup what it started. > Why do we need a copy? When ownership is lost, the context is cleared. In order to re-acquire, an original copy of the context is needed. There is no technical reason to clear the context, so maybe the context should not be cleared after a takeover. Otherwise, many drivers will need to implement the "backup copy" solution. > And why we need to reacquire it? In this particular case the context has disabled interrupts. No other context will re-enable interrupts because the driver is implemented such that the one who disables is the one who enables. So this context must re-acquire ownership in order to re-enable interrupts. > My feeling is that it is needed only to call > console_exit_unsafe(wctxt) later. Or do I miss anything? No. It is only about re-enabling interrupts. The concept of unsafe is not really relevant if a hostile takeover during unsafe occurs. In that case it becomes a "hope and pray" effort at the end of panic(). John ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: locking API: was: [PATCH printk v1 00/18] serial: 8250: implement non-BKL console 2023-03-28 13:57 ` John Ogness @ 2023-03-28 15:10 ` Petr Mladek 2023-03-28 21:47 ` John Ogness 0 siblings, 1 reply; 12+ messages in thread From: Petr Mladek @ 2023-03-28 15:10 UTC (permalink / raw) To: John Ogness Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Jason Wessel, Daniel Thompson, Douglas Anderson, Aaron Tomlin, Luis Chamberlain, kgdb-bugreport, Greg Kroah-Hartman, linux-fsdevel, Andrew Morton, Guilherme G. Piccoli, David Gow, Tiezhu Yang, Daniel Vetter, tangmeng, Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes, rcu On Tue 2023-03-28 16:03:36, John Ogness wrote: > On 2023-03-28, Petr Mladek <pmladek@suse.com> wrote: > >> + if (!__serial8250_clear_IER(up, wctxt, &ier)) > >> + return false; > >> + > >> + if (console_exit_unsafe(wctxt)) { > >> + can_print = atomic_print_line(up, wctxt); > >> + if (!can_print) > >> + atomic_console_reacquire(wctxt, &wctxt_init); > > > > I am trying to review the 9th patch adding console_can_proceed(), > > console_enter_unsafe(), console_exit_unsafe() API. And I wanted > > to see how the struct cons_write_context was actually used. > > First off, I need to post the latest version of the 8250-POC patch. It > is not officially part of this series and is still going through changes > for the PREEMPT_RT tree. I will post the latest version directly after > answering this email. Sure. I know that it is just a kind of POC. > > I am confused now. I do not understand the motivation for the extra > > @wctxt_init copy and atomic_console_reacquire(). > > If an atomic context loses ownership while doing certain activities, it > may need to re-acquire ownership in order to finish or cleanup what it > started. This sounds suspicious. If a console/writer context has lost the lock then all shared/locked resources might already be used by the new owner. I would expect that the context could touch only non-shared resources after loosing the lock. If it re-acquires the lock then the shared resource might be in another state. So, doing any further changes might be dangerous. I could imagine that incrementing/decrementing some counter might make sense but setting some value sounds strange. > > Why do we need a copy? > > When ownership is lost, the context is cleared. In order to re-acquire, > an original copy of the context is needed. There is no technical reason > to clear the context, so maybe the context should not be cleared after a > takeover. Otherwise, many drivers will need to implement the "backup > copy" solution. It might make sense to clear values that are not longer valid, e.g. some state values or .len of the buffer. But I would keep the values that might still be needed to re-acquire the lock. It might be needed when the context want to re-start the entire operation, I guess that you wanted to clean the structure to catch potential misuse. It makes some sense but the copying is really weird. I think that we might/should add some paranoid checks into all functions manipulating the shared state instead. > > And why we need to reacquire it? > > In this particular case the context has disabled interrupts. No other > context will re-enable interrupts because the driver is implemented such > that the one who disables is the one who enables. So this context must > re-acquire ownership in order to re-enable interrupts. My understanding is that the driver might lose the lock only during hostile takeover. Is it safe to re-enable interrupts in this case? Well, it actually might make sense if the interrupts should be enabled when the port is unused. Well, I guess that they will get enabled by the other hostile owner. It should leave the serial port in a good state when it releases the lock a normal way. Anyway, thanks a lot for the info. I still have to scratch my head around this. Best Regards, Petr ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: locking API: was: [PATCH printk v1 00/18] serial: 8250: implement non-BKL console 2023-03-28 15:10 ` Petr Mladek @ 2023-03-28 21:47 ` John Ogness 2023-03-29 8:03 ` Petr Mladek 0 siblings, 1 reply; 12+ messages in thread From: John Ogness @ 2023-03-28 21:47 UTC (permalink / raw) To: Petr Mladek Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Jason Wessel, Daniel Thompson, Douglas Anderson, Aaron Tomlin, Luis Chamberlain, kgdb-bugreport, Greg Kroah-Hartman, linux-fsdevel, Andrew Morton, Guilherme G. Piccoli, David Gow, Tiezhu Yang, Daniel Vetter, tangmeng, Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes, rcu On 2023-03-28, Petr Mladek <pmladek@suse.com> wrote: >> If an atomic context loses ownership while doing certain activities, >> it may need to re-acquire ownership in order to finish or cleanup >> what it started. > > This sounds suspicious. If a console/writer context has lost the lock > then all shared/locked resources might already be used by the new > owner. Correct. > I would expect that the context could touch only non-shared resources > after loosing the lock. Correct. > If it re-acquires the lock then the shared resource might be in > another state. So, doing any further changes might be dangerous. That is the responsibility of the driver to implement it safely if it is needed. > I could imagine that incrementing/decrementing some counter might > make sense but setting some value sounds strange. The 8250 driver must disable interrupts before writing to the TX FIFO. After writing it re-enables the interrupts. However, it might be the case that the interrupts were already disabled, in which case after writing they are left disabled. IOW, whatever context disabled the interrupts is the context that is expected to re-enable them. This simple rule makes it really easy to handle nested printing because a context is only concerned with restoring the IER state that it originally saw. Using counters or passing around interrupt re-enabling responsibility would be considerably trickier. >>> And why we need to reacquire it? >> >> In this particular case the context has disabled interrupts. No other >> context will re-enable interrupts because the driver is implemented >> such that the one who disables is the one who enables. So this >> context must re-acquire ownership in order to re-enable interrupts. > > My understanding is that the driver might lose the lock only > during hostile takeover. Is it safe to re-enable interrupts > in this case? Your understanding is incorrect. If a more important outputting context should arrive, the non-important outputting context will happily and kindly handover to the higher priority. From the perspective of the atomic console driver, it lost ownership. Simple example: The kthread printer is printing and some WARN_ON() is triggered on another CPU. The warning will be output at a higher priority and print from the context/CPU of the WARN_ON(). The kthread printer will lose its ownership by handing over to the warning CPU. Note that we are _not_ talking about when the unsafe bit is set. We are talking about a printer that owns the console, is in a safe section, and loses ownership. If that context was the one that disabled interrupts, it needs to re-acquire the console in order to safely re-enable the interrupts. The context that tookover ownership saw that interrupts are disabled and does _not_ re-enable them when it is finished printing. John ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: locking API: was: [PATCH printk v1 00/18] serial: 8250: implement non-BKL console 2023-03-28 21:47 ` John Ogness @ 2023-03-29 8:03 ` Petr Mladek 0 siblings, 0 replies; 12+ messages in thread From: Petr Mladek @ 2023-03-29 8:03 UTC (permalink / raw) To: John Ogness Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Jason Wessel, Daniel Thompson, Douglas Anderson, Aaron Tomlin, Luis Chamberlain, kgdb-bugreport, Greg Kroah-Hartman, linux-fsdevel, Andrew Morton, Guilherme G. Piccoli, David Gow, Tiezhu Yang, Daniel Vetter, tangmeng, Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes, rcu On Tue 2023-03-28 23:53:16, John Ogness wrote: > On 2023-03-28, Petr Mladek <pmladek@suse.com> wrote: > >> If an atomic context loses ownership while doing certain activities, > >> it may need to re-acquire ownership in order to finish or cleanup > >> what it started. > > > > This sounds suspicious. If a console/writer context has lost the lock > > then all shared/locked resources might already be used by the new > > owner. > > Correct. > > > I would expect that the context could touch only non-shared resources > > after loosing the lock. > > Correct. > > The 8250 driver must disable interrupts before writing to the TX > FIFO. After writing it re-enables the interrupts. However, it might be > the case that the interrupts were already disabled, in which case after > writing they are left disabled. I see. The reacquire() makes sense now. Thanks a lot for explanation. Best Regards, Petr ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH printk v1 00/18] POC: serial: 8250: implement nbcon console 2023-03-02 19:58 ` [PATCH printk v1 00/18] serial: 8250: implement non-BKL console John Ogness 2023-03-28 13:33 ` locking API: was: " Petr Mladek @ 2023-03-28 13:59 ` John Ogness 1 sibling, 0 replies; 12+ messages in thread From: John Ogness @ 2023-03-28 13:59 UTC (permalink / raw) To: Petr Mladek Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Jason Wessel, Daniel Thompson, Douglas Anderson, Aaron Tomlin, Luis Chamberlain, kgdb-bugreport, Greg Kroah-Hartman, linux-fsdevel, Andrew Morton, Guilherme G. Piccoli, David Gow, Tiezhu Yang, Daniel Vetter, tangmeng, Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes, rcu Implement the necessary callbacks to allow the 8250 console driver to perform as a nbcon console. Remove the implementation for the legacy console callback (write) and add implementations for the nbcon consoles (write_atomic, write_thread, port_lock) and add CON_NBCON to the initial flags. Although nbcon consoles can coexist with legacy consoles, you will only receive all the benefits of the nbcon consoles if this console driver is the only console. That means no netconsole, no tty1, no earlyprintk, no earlycon. Just the uart8250. For example: console=ttyS0,115200 Signed-off-by: John Ogness <john.ogness@linutronix.de> --- drivers/tty/serial/8250/8250.h | 269 +++++++++++++- drivers/tty/serial/8250/8250_aspeed_vuart.c | 2 +- drivers/tty/serial/8250/8250_bcm7271.c | 23 +- drivers/tty/serial/8250/8250_core.c | 50 ++- drivers/tty/serial/8250/8250_exar.c | 8 +- drivers/tty/serial/8250/8250_fsl.c | 3 +- drivers/tty/serial/8250/8250_mtk.c | 30 +- drivers/tty/serial/8250/8250_omap.c | 36 +- drivers/tty/serial/8250/8250_port.c | 369 +++++++++++++++----- drivers/tty/serial/8250/Kconfig | 1 + drivers/tty/serial/serial_core.c | 10 +- include/linux/serial_8250.h | 11 +- 12 files changed, 686 insertions(+), 126 deletions(-) diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h index 287153d32536..beb77af4ec62 100644 --- a/drivers/tty/serial/8250/8250.h +++ b/drivers/tty/serial/8250/8250.h @@ -177,12 +177,277 @@ static inline void serial_dl_write(struct uart_8250_port *up, int value) up->dl_write(up, value); } +static inline bool serial8250_is_console(struct uart_port *port) +{ + return uart_console(port) && !hlist_unhashed_lockless(&port->cons->node); +} + +/** + * serial8250_init_wctxt - Initialize a write context for + * non-console-printing usage + * @wctxt: The write context to initialize + * @cons: The console to assign to the write context + * + * In order to mark an unsafe region, drivers must acquire the console. This + * requires providing an initialized write context (even if that driver will + * not be doing any printing). + * + * This function should not be used for console printing contexts. + */ +static inline void serial8250_init_wctxt(struct nbcon_write_context *wctxt, + struct console *cons) +{ + struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt); + + memset(wctxt, 0, sizeof(*wctxt)); + ctxt->console = cons; + ctxt->prio = NBCON_PRIO_NORMAL; +} + +/** + * __serial8250_console_acquire - Acquire a console for + * non-console-printing usage + * @wctxt: An uninitialized write context to use for acquiring + * @cons: The console to assign to the write context + * + * The caller is holding the port->lock. + * The caller is holding the console_srcu_read_lock. + * + * This function should not be used for console printing contexts. + */ +static inline void __serial8250_console_acquire(struct nbcon_write_context *wctxt, + struct console *cons) +{ + for (;;) { + serial8250_init_wctxt(wctxt, cons); + if (nbcon_try_acquire(wctxt)) + break; + cpu_relax(); + } +} + +/** + * serial8250_enter_unsafe - Mark the beginning of an unsafe region for + * non-console-printing usage + * @up: The port that is entering the unsafe state + * + * The caller should ensure @up is a console before calling this function. + * + * The caller is holding the port->lock. + * This function takes the console_srcu_read_lock and becomes owner of the + * console associated with @up. + * + * This function should not be used for console printing contexts. + */ +static inline void serial8250_enter_unsafe(struct uart_8250_port *up) +{ + struct uart_port *port = &up->port; + + lockdep_assert_held_once(&port->lock); + + for (;;) { + up->cookie = console_srcu_read_lock(); + + __serial8250_console_acquire(&up->wctxt, port->cons); + + if (nbcon_enter_unsafe(&up->wctxt)) + break; + + console_srcu_read_unlock(up->cookie); + cpu_relax(); + } +} + +/** + * serial8250_exit_unsafe - Mark the end of an unsafe region for + * non-console-printing usage + * @up: The port that is exiting the unsafe state + * + * The caller is holding the port->lock. + * This function releases ownership of the console associated with @up and + * releases the console_srcu_read_lock. + * + * This function should not be used for console printing contexts. + */ +static inline void serial8250_exit_unsafe(struct uart_8250_port *up) +{ + struct uart_port *port = &up->port; + + lockdep_assert_held_once(&port->lock); + + if (nbcon_exit_unsafe(&up->wctxt)) + nbcon_release(&up->wctxt); + + console_srcu_read_unlock(up->cookie); +} + +/** + * serial8250_in_IER - Read the IER register for + * non-console-printing usage + * @up: The port to work on + * + * Returns: The value read from IER + * + * The caller is holding the port->lock. + * + * This is the top-level function for non-console-printing contexts to + * read the IER register. The caller does not need to care if @up is a + * console before calling this function. + * + * This function should not be used for printing contexts. + */ +static inline int serial8250_in_IER(struct uart_8250_port *up) +{ + struct uart_port *port = &up->port; + bool is_console; + int ier; + + is_console = serial8250_is_console(port); + + if (is_console) + serial8250_enter_unsafe(up); + + ier = serial_in(up, UART_IER); + + if (is_console) + serial8250_exit_unsafe(up); + + return ier; +} + +/** + * __serial8250_set_IER - Directly write to the IER register + * @up: The port to work on + * @wctxt: The current write context + * @ier: The value to write + * + * Returns: True if IER was written to. False otherwise + * + * The caller is holding the port->lock. + * The caller is holding the console_srcu_read_unlock. + * The caller is the owner of the console associated with @up. + * + * This function should only be directly called within console printing + * contexts. Other contexts should use serial8250_set_IER(). + */ +static inline bool __serial8250_set_IER(struct uart_8250_port *up, + struct nbcon_write_context *wctxt, + int ier) +{ + if (wctxt && !nbcon_can_proceed(wctxt)) + return false; + serial_out(up, UART_IER, ier); + return true; +} + +/** + * serial8250_set_IER - Write a new value to the IER register for + * non-console-printing usage + * @up: The port to work on + * @ier: The value to write + * + * The caller is holding the port->lock. + * + * This is the top-level function for non-console-printing contexts to + * write to the IER register. The caller does not need to care if @up is a + * console before calling this function. + * + * This function should not be used for printing contexts. + */ +static inline void serial8250_set_IER(struct uart_8250_port *up, int ier) +{ + struct uart_port *port = &up->port; + bool is_console; + + is_console = serial8250_is_console(port); + + if (is_console) { + serial8250_enter_unsafe(up); + while (!__serial8250_set_IER(up, &up->wctxt, ier)) { + console_srcu_read_unlock(up->cookie); + nbcon_enter_unsafe(&up->wctxt); + } + serial8250_exit_unsafe(up); + } else { + __serial8250_set_IER(up, NULL, ier); + } +} + +/** + * __serial8250_clear_IER - Directly clear the IER register + * @up: The port to work on + * @wctxt: The current write context + * @prior: Gets set to the previous value of IER + * + * Returns: True if IER was cleared and @prior points to the previous + * value of IER. False otherwise and @prior is invalid + * + * The caller is holding the port->lock. + * The caller is holding the console_srcu_read_unlock. + * The caller is the owner of the console associated with @up. + * + * This function should only be directly called within console printing + * contexts. Other contexts should use serial8250_clear_IER(). + */ +static inline bool __serial8250_clear_IER(struct uart_8250_port *up, + struct nbcon_write_context *wctxt, + int *prior) +{ + unsigned int clearval = 0; + + if (up->capabilities & UART_CAP_UUE) + clearval = UART_IER_UUE; + + *prior = serial_in(up, UART_IER); + if (wctxt && !nbcon_can_proceed(wctxt)) + return false; + serial_out(up, UART_IER, clearval); + return true; +} + +/** + * serial8250_clear_IER - Clear the IER register for + * non-console-printing usage + * @up: The port to work on + * + * Returns: The previous value of IER + * + * The caller is holding the port->lock. + * + * This is the top-level function for non-console-printing contexts to + * clear the IER register. The caller does not need to care if @up is a + * console before calling this function. + * + * This function should not be used for printing contexts. + */ +static inline int serial8250_clear_IER(struct uart_8250_port *up) +{ + struct uart_port *port = &up->port; + bool is_console; + int prior; + + is_console = serial8250_is_console(port); + + if (is_console) { + serial8250_enter_unsafe(up); + while (!__serial8250_clear_IER(up, &up->wctxt, &prior)) { + console_srcu_read_unlock(up->cookie); + nbcon_enter_unsafe(&up->wctxt); + } + serial8250_exit_unsafe(up); + } else { + __serial8250_clear_IER(up, NULL, &prior); + } + + return prior; +} + static inline bool serial8250_set_THRI(struct uart_8250_port *up) { if (up->ier & UART_IER_THRI) return false; up->ier |= UART_IER_THRI; - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); return true; } @@ -191,7 +456,7 @@ static inline bool serial8250_clear_THRI(struct uart_8250_port *up) if (!(up->ier & UART_IER_THRI)) return false; up->ier &= ~UART_IER_THRI; - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); return true; } diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c index 9d2a7856784f..7cc6b527c088 100644 --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c @@ -278,7 +278,7 @@ static void __aspeed_vuart_set_throttle(struct uart_8250_port *up, up->ier &= ~irqs; if (!throttle) up->ier |= irqs; - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); } static void aspeed_vuart_set_throttle(struct uart_port *port, bool throttle) { diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c index ed5a94747692..c6f2cd3f19b5 100644 --- a/drivers/tty/serial/8250/8250_bcm7271.c +++ b/drivers/tty/serial/8250/8250_bcm7271.c @@ -606,8 +606,10 @@ static int brcmuart_startup(struct uart_port *port) * Disable the Receive Data Interrupt because the DMA engine * will handle this. */ + spin_lock_irq(&port->lock); up->ier &= ~UART_IER_RDI; - serial_port_out(port, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); + spin_unlock_irq(&port->lock); priv->tx_running = false; priv->dma.rx_dma = NULL; @@ -787,6 +789,12 @@ static int brcmuart_handle_irq(struct uart_port *p) spin_lock_irqsave(&p->lock, flags); status = serial_port_in(p, UART_LSR); if ((status & UART_LSR_DR) == 0) { + bool is_console; + + is_console = serial8250_is_console(p); + + if (is_console) + serial8250_enter_unsafe(up); ier = serial_port_in(p, UART_IER); /* @@ -807,6 +815,9 @@ static int brcmuart_handle_irq(struct uart_port *p) serial_port_in(p, UART_RX); } + if (is_console) + serial8250_exit_unsafe(up); + handled = 1; } spin_unlock_irqrestore(&p->lock, flags); @@ -844,12 +855,22 @@ static enum hrtimer_restart brcmuart_hrtimer_func(struct hrtimer *t) /* re-enable receive unless upper layer has disabled it */ if ((up->ier & (UART_IER_RLSI | UART_IER_RDI)) == (UART_IER_RLSI | UART_IER_RDI)) { + bool is_console; + + is_console = serial8250_is_console(p); + + if (is_console) + serial8250_enter_unsafe(up); + status = serial_port_in(p, UART_IER); status |= (UART_IER_RLSI | UART_IER_RDI); serial_port_out(p, UART_IER, status); status = serial_port_in(p, UART_MCR); status |= UART_MCR_RTS; serial_port_out(p, UART_MCR, status); + + if (is_console) + serial8250_exit_unsafe(up); } spin_unlock_irqrestore(&p->lock, flags); return HRTIMER_NORESTART; diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index ab63c308be0a..6910160a5cec 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -256,6 +256,7 @@ static void serial8250_timeout(struct timer_list *t) static void serial8250_backup_timeout(struct timer_list *t) { struct uart_8250_port *up = from_timer(up, t, timer); + struct uart_port *port = &up->port; unsigned int iir, ier = 0, lsr; unsigned long flags; @@ -266,8 +267,23 @@ static void serial8250_backup_timeout(struct timer_list *t) * based handler. */ if (up->port.irq) { + bool is_console; + + /* + * Do not use serial8250_clear_IER() because this code + * ignores capabilties. + */ + + is_console = serial8250_is_console(port); + + if (is_console) + serial8250_enter_unsafe(up); + ier = serial_in(up, UART_IER); serial_out(up, UART_IER, 0); + + if (is_console) + serial8250_exit_unsafe(up); } iir = serial_in(up, UART_IIR); @@ -290,7 +306,7 @@ static void serial8250_backup_timeout(struct timer_list *t) serial8250_tx_chars(up); if (up->port.irq) - serial_out(up, UART_IER, ier); + serial8250_set_IER(up, ier); spin_unlock_irqrestore(&up->port.lock, flags); @@ -576,12 +592,30 @@ serial8250_register_ports(struct uart_driver *drv, struct device *dev) #ifdef CONFIG_SERIAL_8250_CONSOLE -static void univ8250_console_write(struct console *co, const char *s, - unsigned int count) +static void univ8250_console_port_lock(struct console *con, bool do_lock, unsigned long *flags) +{ + struct uart_8250_port *up = &serial8250_ports[con->index]; + + if (do_lock) + spin_lock_irqsave(&up->port.lock, *flags); + else + spin_unlock_irqrestore(&up->port.lock, *flags); +} + +static bool univ8250_console_write_atomic(struct console *co, + struct nbcon_write_context *wctxt) +{ + struct uart_8250_port *up = &serial8250_ports[co->index]; + + return serial8250_console_write_atomic(up, wctxt); +} + +static bool univ8250_console_write_thread(struct console *co, + struct nbcon_write_context *wctxt) { struct uart_8250_port *up = &serial8250_ports[co->index]; - serial8250_console_write(up, s, count); + return serial8250_console_write_thread(up, wctxt); } static int univ8250_console_setup(struct console *co, char *options) @@ -669,12 +703,14 @@ static int univ8250_console_match(struct console *co, char *name, int idx, static struct console univ8250_console = { .name = "ttyS", - .write = univ8250_console_write, + .write_atomic = univ8250_console_write_atomic, + .write_thread = univ8250_console_write_thread, + .port_lock = univ8250_console_port_lock, .device = uart_console_device, .setup = univ8250_console_setup, .exit = univ8250_console_exit, .match = univ8250_console_match, - .flags = CON_PRINTBUFFER | CON_ANYTIME, + .flags = CON_PRINTBUFFER | CON_ANYTIME | CON_NBCON, .index = -1, .data = &serial8250_reg, }; @@ -962,7 +998,7 @@ static void serial_8250_overrun_backoff_work(struct work_struct *work) spin_lock_irqsave(&port->lock, flags); up->ier |= UART_IER_RLSI | UART_IER_RDI; up->port.read_status_mask |= UART_LSR_DR; - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); spin_unlock_irqrestore(&port->lock, flags); } diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c index 64770c62bbec..ccb70b20b1f4 100644 --- a/drivers/tty/serial/8250/8250_exar.c +++ b/drivers/tty/serial/8250/8250_exar.c @@ -185,6 +185,10 @@ static void xr17v35x_set_divisor(struct uart_port *p, unsigned int baud, static int xr17v35x_startup(struct uart_port *port) { + struct uart_8250_port *up = up_to_u8250p(port); + + spin_lock_irq(&port->lock); + /* * First enable access to IER [7:5], ISR [5:4], FCR [5:4], * MCR [7:5] and MSR [7:0] @@ -195,7 +199,9 @@ static int xr17v35x_startup(struct uart_port *port) * Make sure all interrups are masked until initialization is * complete and the FIFOs are cleared */ - serial_port_out(port, UART_IER, 0); + serial8250_set_IER(up, 0); + + spin_unlock_irq(&port->lock); return serial8250_do_startup(port); } diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c index 8adfaa183f77..eaf148245a10 100644 --- a/drivers/tty/serial/8250/8250_fsl.c +++ b/drivers/tty/serial/8250/8250_fsl.c @@ -58,7 +58,8 @@ int fsl8250_handle_irq(struct uart_port *port) if ((orig_lsr & UART_LSR_OE) && (up->overrun_backoff_time_ms > 0)) { unsigned long delay; - up->ier = port->serial_in(port, UART_IER); + up->ier = serial8250_in_IER(up); + if (up->ier & (UART_IER_RLSI | UART_IER_RDI)) { port->ops->stop_rx(port); } else { diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c index fb1d5ec0940e..bf7ab55c8923 100644 --- a/drivers/tty/serial/8250/8250_mtk.c +++ b/drivers/tty/serial/8250/8250_mtk.c @@ -222,12 +222,38 @@ static void mtk8250_shutdown(struct uart_port *port) static void mtk8250_disable_intrs(struct uart_8250_port *up, int mask) { - serial_out(up, UART_IER, serial_in(up, UART_IER) & (~mask)); + struct uart_port *port = &up->port; + bool is_console; + int ier; + + is_console = serial8250_is_console(port); + + if (is_console) + serial8250_enter_unsafe(up); + + ier = serial_in(up, UART_IER); + serial_out(up, UART_IER, ier & (~mask)); + + if (is_console) + serial8250_exit_unsafe(up); } static void mtk8250_enable_intrs(struct uart_8250_port *up, int mask) { - serial_out(up, UART_IER, serial_in(up, UART_IER) | mask); + struct uart_port *port = &up->port; + bool is_console; + int ier; + + is_console = serial8250_is_console(port); + + if (is_console) + serial8250_enter_unsafe(up); + + ier = serial_in(up, UART_IER); + serial_out(up, UART_IER, ier | mask); + + if (is_console) + serial8250_exit_unsafe(up); } static void mtk8250_set_flow_ctrl(struct uart_8250_port *up, int mode) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 734f092ef839..bfa50a26349d 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -334,8 +334,7 @@ static void omap8250_restore_regs(struct uart_8250_port *up) /* drop TCR + TLR access, we setup XON/XOFF later */ serial8250_out_MCR(up, mcr); - - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); serial_dl_write(up, priv->quot); @@ -523,16 +522,21 @@ static void omap_8250_pm(struct uart_port *port, unsigned int state, u8 efr; pm_runtime_get_sync(port->dev); + + spin_lock_irq(&port->lock); + serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); efr = serial_in(up, UART_EFR); serial_out(up, UART_EFR, efr | UART_EFR_ECB); serial_out(up, UART_LCR, 0); - serial_out(up, UART_IER, (state != 0) ? UART_IERX_SLEEP : 0); + serial8250_set_IER(up, (state != 0) ? UART_IERX_SLEEP : 0); serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); serial_out(up, UART_EFR, efr); serial_out(up, UART_LCR, 0); + spin_unlock_irq(&port->lock); + pm_runtime_mark_last_busy(port->dev); pm_runtime_put_autosuspend(port->dev); } @@ -649,7 +653,8 @@ static irqreturn_t omap8250_irq(int irq, void *dev_id) if ((lsr & UART_LSR_OE) && up->overrun_backoff_time_ms > 0) { unsigned long delay; - up->ier = port->serial_in(port, UART_IER); + spin_lock(&port->lock); + up->ier = serial8250_in_IER(up); if (up->ier & (UART_IER_RLSI | UART_IER_RDI)) { port->ops->stop_rx(port); } else { @@ -658,6 +663,7 @@ static irqreturn_t omap8250_irq(int irq, void *dev_id) */ cancel_delayed_work(&up->overrun_backoff); } + spin_unlock(&port->lock); delay = msecs_to_jiffies(up->overrun_backoff_time_ms); schedule_delayed_work(&up->overrun_backoff, delay); @@ -707,8 +713,10 @@ static int omap_8250_startup(struct uart_port *port) if (ret < 0) goto err; + spin_lock_irq(&port->lock); up->ier = UART_IER_RLSI | UART_IER_RDI; - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); + spin_unlock_irq(&port->lock); #ifdef CONFIG_PM up->capabilities |= UART_CAP_RPM; @@ -748,8 +756,10 @@ static void omap_8250_shutdown(struct uart_port *port) if (priv->habit & UART_HAS_EFR2) serial_out(up, UART_OMAP_EFR2, 0x0); + spin_lock_irq(&port->lock); up->ier = 0; - serial_out(up, UART_IER, 0); + serial8250_set_IER(up, 0); + spin_unlock_irq(&port->lock); if (up->dma) serial8250_release_dma(up); @@ -797,7 +807,7 @@ static void omap_8250_unthrottle(struct uart_port *port) up->dma->rx_dma(up); up->ier |= UART_IER_RLSI | UART_IER_RDI; port->read_status_mask |= UART_LSR_DR; - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); spin_unlock_irqrestore(&port->lock, flags); pm_runtime_mark_last_busy(port->dev); @@ -956,7 +966,7 @@ static void __dma_rx_complete(void *param) __dma_rx_do_complete(p); if (!priv->throttled) { p->ier |= UART_IER_RLSI | UART_IER_RDI; - serial_out(p, UART_IER, p->ier); + serial8250_set_IER(p, p->ier); if (!(priv->habit & UART_HAS_EFR2)) omap_8250_rx_dma(p); } @@ -1013,7 +1023,7 @@ static int omap_8250_rx_dma(struct uart_8250_port *p) * callback to run. */ p->ier &= ~(UART_IER_RLSI | UART_IER_RDI); - serial_out(p, UART_IER, p->ier); + serial8250_set_IER(p, p->ier); } goto out; } @@ -1226,12 +1236,12 @@ static void am654_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir, * periodic timeouts, re-enable interrupts. */ up->ier &= ~(UART_IER_RLSI | UART_IER_RDI); - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); omap_8250_rx_dma_flush(up); serial_in(up, UART_IIR); serial_out(up, UART_OMAP_EFR2, 0x0); up->ier |= UART_IER_RLSI | UART_IER_RDI; - serial_out(up, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); } } @@ -1717,12 +1727,16 @@ static int omap8250_runtime_resume(struct device *dev) up = serial8250_get_port(priv->line); + spin_lock_irq(&up->port.lock); + if (omap8250_lost_context(up)) omap8250_restore_regs(up); if (up->dma && up->dma->rxchan && !(priv->habit & UART_HAS_EFR2)) omap_8250_rx_dma(up); + spin_unlock_irq(&up->port.lock); + priv->latency = priv->calc_latency; schedule_work(&priv->qos_work); return 0; diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index fa43df05342b..4378349862e6 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -744,6 +744,7 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) serial8250_rpm_get(p); if (p->capabilities & UART_CAP_SLEEP) { + spin_lock_irq(&p->port.lock); if (p->capabilities & UART_CAP_EFR) { lcr = serial_in(p, UART_LCR); efr = serial_in(p, UART_EFR); @@ -751,25 +752,18 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) serial_out(p, UART_EFR, UART_EFR_ECB); serial_out(p, UART_LCR, 0); } - serial_out(p, UART_IER, sleep ? UART_IERX_SLEEP : 0); + serial8250_set_IER(p, sleep ? UART_IERX_SLEEP : 0); if (p->capabilities & UART_CAP_EFR) { serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B); serial_out(p, UART_EFR, efr); serial_out(p, UART_LCR, lcr); } + spin_unlock_irq(&p->port.lock); } serial8250_rpm_put(p); } -static void serial8250_clear_IER(struct uart_8250_port *up) -{ - if (up->capabilities & UART_CAP_UUE) - serial_out(up, UART_IER, UART_IER_UUE); - else - serial_out(up, UART_IER, 0); -} - #ifdef CONFIG_SERIAL_8250_RSA /* * Attempts to turn on the RSA FIFO. Returns zero on failure. @@ -1033,8 +1027,10 @@ static int broken_efr(struct uart_8250_port *up) */ static void autoconfig_16550a(struct uart_8250_port *up) { + struct uart_port *port = &up->port; unsigned char status1, status2; unsigned int iersave; + bool is_console; up->port.type = PORT_16550A; up->capabilities |= UART_CAP_FIFO; @@ -1150,6 +1146,11 @@ static void autoconfig_16550a(struct uart_8250_port *up) return; } + is_console = serial8250_is_console(port); + + if (is_console) + serial8250_enter_unsafe(up); + /* * Try writing and reading the UART_IER_UUE bit (b6). * If it works, this is probably one of the Xscale platform's @@ -1185,6 +1186,9 @@ static void autoconfig_16550a(struct uart_8250_port *up) } serial_out(up, UART_IER, iersave); + if (is_console) + serial8250_exit_unsafe(up); + /* * We distinguish between 16550A and U6 16550A by counting * how many bytes are in the FIFO. @@ -1226,6 +1230,13 @@ static void autoconfig(struct uart_8250_port *up) up->bugs = 0; if (!(port->flags & UPF_BUGGY_UART)) { + bool is_console; + + is_console = serial8250_is_console(port); + + if (is_console) + serial8250_enter_unsafe(up); + /* * Do a simple existence test first; if we fail this, * there's no point trying anything else. @@ -1255,6 +1266,10 @@ static void autoconfig(struct uart_8250_port *up) #endif scratch3 = serial_in(up, UART_IER) & UART_IER_ALL_INTR; serial_out(up, UART_IER, scratch); + + if (is_console) + serial8250_exit_unsafe(up); + if (scratch2 != 0 || scratch3 != UART_IER_ALL_INTR) { /* * We failed; there's nothing here @@ -1376,6 +1391,7 @@ static void autoconfig_irq(struct uart_8250_port *up) unsigned char save_ICP = 0; unsigned int ICP = 0; unsigned long irqs; + bool is_console; int irq; if (port->flags & UPF_FOURPORT) { @@ -1385,8 +1401,12 @@ static void autoconfig_irq(struct uart_8250_port *up) inb_p(ICP); } - if (uart_console(port)) + is_console = serial8250_is_console(port); + + if (is_console) { console_lock(); + serial8250_enter_unsafe(up); + } /* forget possible initially masked and pending IRQ */ probe_irq_off(probe_irq_on()); @@ -1418,8 +1438,10 @@ static void autoconfig_irq(struct uart_8250_port *up) if (port->flags & UPF_FOURPORT) outb_p(save_ICP, ICP); - if (uart_console(port)) + if (is_console) { + serial8250_exit_unsafe(up); console_unlock(); + } port->irq = (irq > 0) ? irq : 0; } @@ -1432,7 +1454,7 @@ static void serial8250_stop_rx(struct uart_port *port) up->ier &= ~(UART_IER_RLSI | UART_IER_RDI); up->port.read_status_mask &= ~UART_LSR_DR; - serial_port_out(port, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); serial8250_rpm_put(up); } @@ -1462,7 +1484,7 @@ void serial8250_em485_stop_tx(struct uart_8250_port *p) serial8250_clear_and_reinit_fifos(p); p->ier |= UART_IER_RLSI | UART_IER_RDI; - serial_port_out(&p->port, UART_IER, p->ier); + serial8250_set_IER(p, p->ier); } } EXPORT_SYMBOL_GPL(serial8250_em485_stop_tx); @@ -1709,7 +1731,7 @@ static void serial8250_disable_ms(struct uart_port *port) mctrl_gpio_disable_ms(up->gpios); up->ier &= ~UART_IER_MSI; - serial_port_out(port, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); } static void serial8250_enable_ms(struct uart_port *port) @@ -1725,7 +1747,7 @@ static void serial8250_enable_ms(struct uart_port *port) up->ier |= UART_IER_MSI; serial8250_rpm_get(up); - serial_port_out(port, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); serial8250_rpm_put(up); } @@ -2160,9 +2182,10 @@ static void serial8250_put_poll_char(struct uart_port *port, serial8250_rpm_get(up); /* * First save the IER then disable the interrupts + * + * Best-effort IER access because other CPUs are quiesced. */ - ier = serial_port_in(port, UART_IER); - serial8250_clear_IER(up); + __serial8250_clear_IER(up, NULL, &ier); wait_for_xmitr(up, UART_LSR_BOTH_EMPTY); /* @@ -2175,7 +2198,7 @@ static void serial8250_put_poll_char(struct uart_port *port, * and restore the IER */ wait_for_xmitr(up, UART_LSR_BOTH_EMPTY); - serial_port_out(port, UART_IER, ier); + __serial8250_set_IER(up, NULL, ier); serial8250_rpm_put(up); } @@ -2186,6 +2209,7 @@ int serial8250_do_startup(struct uart_port *port) struct uart_8250_port *up = up_to_u8250p(port); unsigned long flags; unsigned char iir; + bool is_console; int retval; u16 lsr; @@ -2203,21 +2227,25 @@ int serial8250_do_startup(struct uart_port *port) serial8250_rpm_get(up); if (port->type == PORT_16C950) { /* Wake up and initialize UART */ + spin_lock_irqsave(&port->lock, flags); up->acr = 0; serial_port_out(port, UART_LCR, UART_LCR_CONF_MODE_B); serial_port_out(port, UART_EFR, UART_EFR_ECB); - serial_port_out(port, UART_IER, 0); + serial8250_set_IER(up, 0); serial_port_out(port, UART_LCR, 0); serial_icr_write(up, UART_CSR, 0); /* Reset the UART */ serial_port_out(port, UART_LCR, UART_LCR_CONF_MODE_B); serial_port_out(port, UART_EFR, UART_EFR_ECB); serial_port_out(port, UART_LCR, 0); + spin_unlock_irqrestore(&port->lock, flags); } if (port->type == PORT_DA830) { /* Reset the port */ - serial_port_out(port, UART_IER, 0); + spin_lock_irqsave(&port->lock, flags); + serial8250_set_IER(up, 0); serial_port_out(port, UART_DA830_PWREMU_MGMT, 0); + spin_unlock_irqrestore(&port->lock, flags); mdelay(10); /* Enable Tx, Rx and free run mode */ @@ -2315,6 +2343,8 @@ int serial8250_do_startup(struct uart_port *port) if (retval) goto out; + is_console = serial8250_is_console(port); + if (port->irq && !(up->port.flags & UPF_NO_THRE_TEST)) { unsigned char iir1; @@ -2331,6 +2361,9 @@ int serial8250_do_startup(struct uart_port *port) */ spin_lock_irqsave(&port->lock, flags); + if (is_console) + serial8250_enter_unsafe(up); + wait_for_xmitr(up, UART_LSR_THRE); serial_port_out_sync(port, UART_IER, UART_IER_THRI); udelay(1); /* allow THRE to set */ @@ -2341,6 +2374,9 @@ int serial8250_do_startup(struct uart_port *port) iir = serial_port_in(port, UART_IIR); serial_port_out(port, UART_IER, 0); + if (is_console) + serial8250_exit_unsafe(up); + spin_unlock_irqrestore(&port->lock, flags); if (port->irqflags & IRQF_SHARED) @@ -2395,10 +2431,14 @@ int serial8250_do_startup(struct uart_port *port) * Do a quick test to see if we receive an interrupt when we enable * the TX irq. */ + if (is_console) + serial8250_enter_unsafe(up); serial_port_out(port, UART_IER, UART_IER_THRI); lsr = serial_port_in(port, UART_LSR); iir = serial_port_in(port, UART_IIR); serial_port_out(port, UART_IER, 0); + if (is_console) + serial8250_exit_unsafe(up); if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT) { if (!(up->bugs & UART_BUG_TXEN)) { @@ -2430,7 +2470,7 @@ int serial8250_do_startup(struct uart_port *port) if (up->dma) { const char *msg = NULL; - if (uart_console(port)) + if (is_console) msg = "forbid DMA for kernel console"; else if (serial8250_request_dma(up)) msg = "failed to request DMA"; @@ -2481,7 +2521,7 @@ void serial8250_do_shutdown(struct uart_port *port) */ spin_lock_irqsave(&port->lock, flags); up->ier = 0; - serial_port_out(port, UART_IER, 0); + serial8250_set_IER(up, 0); spin_unlock_irqrestore(&port->lock, flags); synchronize_irq(port->irq); @@ -2847,7 +2887,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios, if (up->capabilities & UART_CAP_RTOIE) up->ier |= UART_IER_RTOIE; - serial_port_out(port, UART_IER, up->ier); + serial8250_set_IER(up, up->ier); if (up->capabilities & UART_CAP_EFR) { unsigned char efr = 0; @@ -3312,12 +3352,21 @@ EXPORT_SYMBOL_GPL(serial8250_set_defaults); #ifdef CONFIG_SERIAL_8250_CONSOLE -static void serial8250_console_putchar(struct uart_port *port, unsigned char ch) +static bool serial8250_console_putchar(struct uart_port *port, unsigned char ch, + struct nbcon_write_context *wctxt) { struct uart_8250_port *up = up_to_u8250p(port); wait_for_xmitr(up, UART_LSR_THRE); + if (!nbcon_can_proceed(wctxt)) + return false; serial_port_out(port, UART_TX, ch); + if (ch == '\n') + up->console_newline_needed = false; + else + up->console_newline_needed = true; + + return true; } /* @@ -3346,33 +3395,119 @@ static void serial8250_console_restore(struct uart_8250_port *up) serial8250_out_MCR(up, up->mcr | UART_MCR_DTR | UART_MCR_RTS); } -/* - * Print a string to the serial port using the device FIFO - * - * It sends fifosize bytes and then waits for the fifo - * to get empty. - */ -static void serial8250_console_fifo_write(struct uart_8250_port *up, - const char *s, unsigned int count) +static bool __serial8250_console_write(struct uart_port *port, struct nbcon_write_context *wctxt, + const char *s, unsigned int count, + bool (*putchar)(struct uart_port *, unsigned char, struct nbcon_write_context *)) { - int i; - const char *end = s + count; - unsigned int fifosize = up->tx_loadsz; - bool cr_sent = false; - - while (s != end) { - wait_for_lsr(up, UART_LSR_THRE); - - for (i = 0; i < fifosize && s != end; ++i) { - if (*s == '\n' && !cr_sent) { - serial_out(up, UART_TX, '\r'); - cr_sent = true; - } else { - serial_out(up, UART_TX, *s++); - cr_sent = false; - } + bool finished = false; + unsigned int i; + + for (i = 0; i < count; i++, s++) { + if (*s == '\n') { + if (!putchar(port, '\r', wctxt)) + goto out; + } + if (!putchar(port, *s, wctxt)) + goto out; + } + finished = true; +out: + return finished; +} + +static bool serial8250_console_write(struct uart_port *port, struct nbcon_write_context *wctxt, + const char *s, unsigned int count, + bool (*putchar)(struct uart_port *, unsigned char, struct nbcon_write_context *)) +{ + return __serial8250_console_write(port, wctxt, s, count, putchar); +} + +static bool atomic_print_line(struct uart_8250_port *up, + struct nbcon_write_context *wctxt) +{ + struct uart_port *port = &up->port; + + if (up->console_newline_needed && + !__serial8250_console_write(port, wctxt, "\n", 1, serial8250_console_putchar)) { + return false; + } + + return __serial8250_console_write(port, wctxt, wctxt->outbuf, wctxt->len, + serial8250_console_putchar); +} + +static void atomic_console_reacquire(struct nbcon_write_context *wctxt, + struct nbcon_write_context *wctxt_init) +{ + memcpy(wctxt, wctxt_init, sizeof(*wctxt)); + while (!nbcon_try_acquire(wctxt)) { + cpu_relax(); + memcpy(wctxt, wctxt_init, sizeof(*wctxt)); + } +} + +bool serial8250_console_write_atomic(struct uart_8250_port *up, + struct nbcon_write_context *wctxt) +{ + struct nbcon_write_context wctxt_init = { }; + struct nbcon_context *ctxt_init = &ACCESS_PRIVATE(&wctxt_init, ctxt); + struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt); + bool finished = false; + unsigned int ier; + + touch_nmi_watchdog(); + + /* With write_atomic, another context may hold the port->lock. */ + + ctxt_init->console = ctxt->console; + ctxt_init->prio = ctxt->prio; + ctxt_init->thread = ctxt->thread; + + /* + * Enter unsafe in order to disable interrupts. If the console is + * lost before the interrupts are disabled, bail out because another + * context took over the printing. If the console is lost after the + * interrutps are disabled, the console must be reacquired in order + * to re-enable the interrupts. However in that case no printing is + * allowed because another context took over the printing. + */ + + if (!nbcon_enter_unsafe(wctxt)) + return false; + + if (!__serial8250_clear_IER(up, wctxt, &ier)) + return false; + + if (!nbcon_exit_unsafe(wctxt)) { + atomic_console_reacquire(wctxt, &wctxt_init); + goto enable_irq; + } + + if (!atomic_print_line(up, wctxt)) { + atomic_console_reacquire(wctxt, &wctxt_init); + goto enable_irq; + } + + wait_for_xmitr(up, UART_LSR_BOTH_EMPTY); + finished = true; +enable_irq: + /* + * Enter unsafe in order to enable interrupts. If the console is + * lost before the interrupts are enabled, the console must be + * reacquired in order to re-enable the interrupts. + */ + for (;;) { + if (nbcon_enter_unsafe(wctxt) && + __serial8250_set_IER(up, wctxt, ier)) { + break; } + + /* HW-IRQs still disabled. Reacquire to enable them. */ + atomic_console_reacquire(wctxt, &wctxt_init); } + nbcon_exit_unsafe(wctxt); + + return finished; } /* @@ -3384,78 +3519,116 @@ static void serial8250_console_fifo_write(struct uart_8250_port *up, * Doing runtime PM is really a bad idea for the kernel console. * Thus, we assume the function is called when device is powered up. */ -void serial8250_console_write(struct uart_8250_port *up, const char *s, - unsigned int count) +bool serial8250_console_write_thread(struct uart_8250_port *up, + struct nbcon_write_context *wctxt) { + struct nbcon_write_context wctxt_init = { }; + struct nbcon_context *ctxt_init = &ACCESS_PRIVATE(&wctxt_init, ctxt); + struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt); struct uart_8250_em485 *em485 = up->em485; struct uart_port *port = &up->port; - unsigned long flags; - unsigned int ier, use_fifo; - int locked = 1; - - touch_nmi_watchdog(); + unsigned int count = wctxt->len; + const char *s = wctxt->outbuf; + bool rs485_started = false; + bool finished = false; + unsigned int ier; - if (oops_in_progress) - locked = spin_trylock_irqsave(&port->lock, flags); - else - spin_lock_irqsave(&port->lock, flags); + ctxt_init->console = ctxt->console; + ctxt_init->prio = ctxt->prio; + ctxt_init->thread = ctxt->thread; /* - * First save the IER then disable the interrupts + * Enter unsafe in order to disable interrupts. If the console is + * lost before the interrupts are disabled, bail out because another + * context took over the printing. If the console is lost after the + * interrutps are disabled, the console must be reacquired in order + * to re-enable the interrupts. However in that case no printing is + * allowed because another context took over the printing. */ - ier = serial_port_in(port, UART_IER); - serial8250_clear_IER(up); + + if (!nbcon_enter_unsafe(wctxt)) + return false; + + if (!__serial8250_clear_IER(up, wctxt, &ier)) + return false; + + if (!nbcon_exit_unsafe(wctxt)) { + atomic_console_reacquire(wctxt, &wctxt_init); + goto enable_irq; + } /* check scratch reg to see if port powered off during system sleep */ if (up->canary && (up->canary != serial_port_in(port, UART_SCR))) { + if (!nbcon_enter_unsafe(wctxt)) { + atomic_console_reacquire(wctxt, &wctxt_init); + goto enable_irq; + } serial8250_console_restore(up); + if (!nbcon_exit_unsafe(wctxt)) { + atomic_console_reacquire(wctxt, &wctxt_init); + goto enable_irq; + } up->canary = 0; } if (em485) { - if (em485->tx_stopped) + if (em485->tx_stopped) { + if (!nbcon_enter_unsafe(wctxt)) { + atomic_console_reacquire(wctxt, &wctxt_init); + goto enable_irq; + } up->rs485_start_tx(up); - mdelay(port->rs485.delay_rts_before_send); + rs485_started = true; + if (!nbcon_exit_unsafe(wctxt)) { + atomic_console_reacquire(wctxt, &wctxt_init); + goto enable_irq; + } + } + if (port->rs485.delay_rts_before_send) { + mdelay(port->rs485.delay_rts_before_send); + if (!nbcon_can_proceed(wctxt)) { + atomic_console_reacquire(wctxt, &wctxt_init); + goto enable_irq; + } + } } - use_fifo = (up->capabilities & UART_CAP_FIFO) && - /* - * BCM283x requires to check the fifo - * after each byte. - */ - !(up->capabilities & UART_CAP_MINI) && - /* - * tx_loadsz contains the transmit fifo size - */ - up->tx_loadsz > 1 && - (up->fcr & UART_FCR_ENABLE_FIFO) && - port->state && - test_bit(TTY_PORT_INITIALIZED, &port->state->port.iflags) && - /* - * After we put a data in the fifo, the controller will send - * it regardless of the CTS state. Therefore, only use fifo - * if we don't use control flow. - */ - !(up->port.flags & UPF_CONS_FLOW); - - if (likely(use_fifo)) - serial8250_console_fifo_write(up, s, count); - else - uart_console_write(port, s, count, serial8250_console_putchar); + if (!serial8250_console_write(port, wctxt, s, count, serial8250_console_putchar)) { + atomic_console_reacquire(wctxt, &wctxt_init); + goto enable_irq; + } + wait_for_xmitr(up, UART_LSR_BOTH_EMPTY); + finished = true; +enable_irq: /* - * Finally, wait for transmitter to become empty - * and restore the IER + * Enter unsafe in order to stop rs485_tx. If the console is + * lost before the rs485_tx is stopped, the console must be + * reacquired in order to stop rs485_tx. */ - wait_for_xmitr(up, UART_LSR_BOTH_EMPTY); - if (em485) { mdelay(port->rs485.delay_rts_after_send); - if (em485->tx_stopped) + if (em485->tx_stopped && rs485_started) { + while (!nbcon_enter_unsafe(wctxt)) + atomic_console_reacquire(wctxt, &wctxt_init); up->rs485_stop_tx(up); + if (!nbcon_exit_unsafe(wctxt)) + atomic_console_reacquire(wctxt, &wctxt_init); + } } - serial_port_out(port, UART_IER, ier); + /* + * Enter unsafe in order to enable interrupts. If the console is + * lost before the interrupts are enabled, the console must be + * reacquired in order to re-enable the interrupts. + */ + for (;;) { + if (nbcon_enter_unsafe(wctxt) && + __serial8250_set_IER(up, wctxt, ier)) { + break; + } + atomic_console_reacquire(wctxt, &wctxt_init); + } /* * The receive handling will happen properly because the @@ -3467,8 +3640,9 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s, if (up->msr_saved_flags) serial8250_modem_status(up); - if (locked) - spin_unlock_irqrestore(&port->lock, flags); + nbcon_exit_unsafe(wctxt); + + return finished; } static unsigned int probe_baud(struct uart_port *port) @@ -3488,6 +3662,7 @@ static unsigned int probe_baud(struct uart_port *port) int serial8250_console_setup(struct uart_port *port, char *options, bool probe) { + struct uart_8250_port *up = up_to_u8250p(port); int baud = 9600; int bits = 8; int parity = 'n'; @@ -3497,6 +3672,8 @@ int serial8250_console_setup(struct uart_port *port, char *options, bool probe) if (!port->iobase && !port->membase) return -ENODEV; + up->console_newline_needed = false; + if (options) uart_parse_options(options, &baud, &parity, &bits, &flow); else if (probe) diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig index 5313aa31930f..16715f01bdb5 100644 --- a/drivers/tty/serial/8250/Kconfig +++ b/drivers/tty/serial/8250/Kconfig @@ -9,6 +9,7 @@ config SERIAL_8250 depends on !S390 select SERIAL_CORE select SERIAL_MCTRL_GPIO if GPIOLIB + select HAVE_ATOMIC_CONSOLE help This selects whether you want to include the driver for the standard serial ports. The standard answer is Y. People who might say N diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 2bd32c8ece39..9901f916dc1a 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -2336,8 +2336,11 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport) * able to Re-start_rx later. */ if (!console_suspend_enabled && uart_console(uport)) { - if (uport->ops->start_rx) + if (uport->ops->start_rx) { + spin_lock_irq(&uport->lock); uport->ops->stop_rx(uport); + spin_unlock_irq(&uport->lock); + } goto unlock; } @@ -2430,8 +2433,11 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport) if (console_suspend_enabled) uart_change_pm(state, UART_PM_STATE_ON); uport->ops->set_termios(uport, &termios, NULL); - if (!console_suspend_enabled && uport->ops->start_rx) + if (!console_suspend_enabled && uport->ops->start_rx) { + spin_lock_irq(&uport->lock); uport->ops->start_rx(uport); + spin_unlock_irq(&uport->lock); + } if (console_suspend_enabled) console_start(uport->cons); } diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h index 19376bee9667..cf73a99232d4 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -125,6 +125,8 @@ struct uart_8250_port { #define MSR_SAVE_FLAGS UART_MSR_ANY_DELTA unsigned char msr_saved_flags; + bool console_newline_needed; + struct uart_8250_dma *dma; const struct uart_8250_ops *ops; @@ -139,6 +141,9 @@ struct uart_8250_port { /* Serial port overrun backoff */ struct delayed_work overrun_backoff; u32 overrun_backoff_time_ms; + + struct nbcon_write_context wctxt; + int cookie; }; static inline struct uart_8250_port *up_to_u8250p(struct uart_port *up) @@ -178,8 +183,10 @@ void serial8250_tx_chars(struct uart_8250_port *up); unsigned int serial8250_modem_status(struct uart_8250_port *up); void serial8250_init_port(struct uart_8250_port *up); void serial8250_set_defaults(struct uart_8250_port *up); -void serial8250_console_write(struct uart_8250_port *up, const char *s, - unsigned int count); +bool serial8250_console_write_atomic(struct uart_8250_port *up, + struct nbcon_write_context *wctxt); +bool serial8250_console_write_thread(struct uart_8250_port *up, + struct nbcon_write_context *wctxt); int serial8250_console_setup(struct uart_port *port, char *options, bool probe); int serial8250_console_exit(struct uart_port *port); -- 2.30.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH printk v1 00/18] threaded/atomic console support 2023-03-02 19:56 [PATCH printk v1 00/18] threaded/atomic console support John Ogness 2023-03-02 19:56 ` [PATCH printk v1 17/18] rcu: Add atomic write enforcement for rcu stalls John Ogness 2023-03-02 19:58 ` [PATCH printk v1 00/18] serial: 8250: implement non-BKL console John Ogness @ 2023-03-09 10:55 ` Daniel Thompson 2023-03-09 11:14 ` John Ogness 2 siblings, 1 reply; 12+ messages in thread From: Daniel Thompson @ 2023-03-09 10:55 UTC (permalink / raw) To: John Ogness Cc: Petr Mladek, Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Jason Wessel, Douglas Anderson, Aaron Tomlin, Luis Chamberlain, kgdb-bugreport, Greg Kroah-Hartman, linux-fsdevel, Andrew Morton, Guilherme G. Piccoli, David Gow, Tiezhu Yang, Daniel Vetter, tangmeng, Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes, rcu On Thu, Mar 02, 2023 at 09:02:00PM +0106, John Ogness wrote: > Hi, > > This is v1 of a series to bring in a new threaded/atomic console > infrastructure. The history, motivation, and various explanations and > examples are available in the cover letter of tglx's RFC series > [0]. From that series, patches 1-18 have been mainlined as of the 6.3 > merge window. What remains, patches 19-29, is what this series > represents. So I grabbed the whole series and pointed it at the kgdb test suite. Don't get too excited about that (the test suite only exercises 8250 and PL011... and IIUC little in the set should impact UART polling anyway) but FWIW: Tested-by: Daniel Thompson <daniel.thompson@linaro.org> Daniel. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH printk v1 00/18] threaded/atomic console support 2023-03-09 10:55 ` [PATCH printk v1 00/18] threaded/atomic console support Daniel Thompson @ 2023-03-09 11:14 ` John Ogness 0 siblings, 0 replies; 12+ messages in thread From: John Ogness @ 2023-03-09 11:14 UTC (permalink / raw) To: Daniel Thompson Cc: Petr Mladek, Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Jason Wessel, Douglas Anderson, Aaron Tomlin, Luis Chamberlain, kgdb-bugreport, Greg Kroah-Hartman, linux-fsdevel, Andrew Morton, Guilherme G. Piccoli, David Gow, Tiezhu Yang, Daniel Vetter, tangmeng, Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes, rcu On 2023-03-09, Daniel Thompson <daniel.thompson@linaro.org> wrote: > So I grabbed the whole series and pointed it at the kgdb test suite. > > Don't get too excited about that (the test suite only exercises 8250 > and PL011... and IIUC little in the set should impact UART polling > anyway) but FWIW: > > Tested-by: Daniel Thompson <daniel.thompson@linaro.org> One of the claims of this series is that it does not break any existing drivers/infrastructure. So any successful test results are certainly of value. John ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-04-13 12:10 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-02 19:56 [PATCH printk v1 00/18] threaded/atomic console support John Ogness 2023-03-02 19:56 ` [PATCH printk v1 17/18] rcu: Add atomic write enforcement for rcu stalls John Ogness 2023-04-13 12:10 ` Petr Mladek 2023-03-02 19:58 ` [PATCH printk v1 00/18] serial: 8250: implement non-BKL console John Ogness 2023-03-28 13:33 ` locking API: was: " Petr Mladek 2023-03-28 13:57 ` John Ogness 2023-03-28 15:10 ` Petr Mladek 2023-03-28 21:47 ` John Ogness 2023-03-29 8:03 ` Petr Mladek 2023-03-28 13:59 ` [PATCH printk v1 00/18] POC: serial: 8250: implement nbcon console John Ogness 2023-03-09 10:55 ` [PATCH printk v1 00/18] threaded/atomic console support Daniel Thompson 2023-03-09 11:14 ` John Ogness
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).