diff options
author | John Ogness <john.ogness@linutronix.de> | 2023-05-25 11:37:59 +0206 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2023-05-30 11:45:42 +0100 |
commit | d0b309a5d3f4648fd4fb1a255a7ae9fb98d87be3 (patch) | |
tree | 89d0491d91746e06337ec8e8fc0a1172a7f95494 | |
parent | 25614735a647693c1260f253dc3ab32127697806 (diff) |
serial: 8250: synchronize and annotate UART_IER access
The UART_IER register is modified twice by each console write
(serial8250_console_write()) under the port lock. Any driver code that
accesses UART_IER must do so with the port locked in order to ensure
consistent values, even when for read accesses.
Add locking, lockdep notation, and/or comments everywhere UART_IER is
accessed. The added locking is not fixing a real problem because it
occurs where the console is not active. However, adding the locking
to these non-critical paths greatly simplifies UART_IER access
tracking by establishing a general policy that all UART_IER access
is performed with the port locked.
Signed-off-by: John Ogness <john.ogness@linutronix.de>
Link: https://lore.kernel.org/r/20230525093159.223817-9-john.ogness@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r-- | drivers/tty/serial/8250/8250.h | 6 | ||||
-rw-r--r-- | drivers/tty/serial/8250/8250_aspeed_vuart.c | 3 | ||||
-rw-r--r-- | drivers/tty/serial/8250/8250_mtk.c | 9 | ||||
-rw-r--r-- | drivers/tty/serial/8250/8250_omap.c | 14 | ||||
-rw-r--r-- | drivers/tty/serial/8250/8250_port.c | 53 |
5 files changed, 85 insertions, 0 deletions
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h index 5418708f4631..471c6bc5f78f 100644 --- a/drivers/tty/serial/8250/8250.h +++ b/drivers/tty/serial/8250/8250.h @@ -179,6 +179,9 @@ static inline void serial_dl_write(struct uart_8250_port *up, u32 value) static inline bool serial8250_set_THRI(struct uart_8250_port *up) { + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + if (up->ier & UART_IER_THRI) return false; up->ier |= UART_IER_THRI; @@ -188,6 +191,9 @@ static inline bool serial8250_set_THRI(struct uart_8250_port *up) static inline bool serial8250_clear_THRI(struct uart_8250_port *up) { + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + if (!(up->ier & UART_IER_THRI)) return false; up->ier &= ~UART_IER_THRI; diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c index 9d2a7856784f..4a9e71b2dbbc 100644 --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c @@ -275,6 +275,9 @@ static void __aspeed_vuart_set_throttle(struct uart_8250_port *up, { unsigned char irqs = UART_IER_RLSI | UART_IER_RDI; + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + up->ier &= ~irqs; if (!throttle) up->ier |= irqs; diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c index fb1d5ec0940e..aa8e98164d68 100644 --- a/drivers/tty/serial/8250/8250_mtk.c +++ b/drivers/tty/serial/8250/8250_mtk.c @@ -222,11 +222,17 @@ static void mtk8250_shutdown(struct uart_port *port) static void mtk8250_disable_intrs(struct uart_8250_port *up, int mask) { + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + serial_out(up, UART_IER, serial_in(up, UART_IER) & (~mask)); } static void mtk8250_enable_intrs(struct uart_8250_port *up, int mask) { + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + serial_out(up, UART_IER, serial_in(up, UART_IER) | mask); } @@ -235,6 +241,9 @@ static void mtk8250_set_flow_ctrl(struct uart_8250_port *up, int mode) struct uart_port *port = &up->port; int lcr = serial_in(up, UART_LCR); + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&port->lock); + serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); serial_out(up, MTK_UART_EFR, UART_EFR_ECB); serial_out(up, UART_LCR, lcr); diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 3225c95fde1d..0498b9b0e4e9 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -533,6 +533,10 @@ static void omap_8250_pm(struct uart_port *port, unsigned int state, u8 efr; pm_runtime_get_sync(port->dev); + + /* Synchronize UART_IER access against the console. */ + 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); @@ -543,6 +547,8 @@ static void omap_8250_pm(struct uart_port *port, unsigned int state, 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); } @@ -760,8 +766,11 @@ static void omap_8250_shutdown(struct uart_port *port) if (priv->habit & UART_HAS_EFR2) serial_out(up, UART_OMAP_EFR2, 0x0); + /* Synchronize UART_IER access against the console. */ + spin_lock_irq(&port->lock); up->ier = 0; serial_out(up, UART_IER, 0); + spin_unlock_irq(&port->lock); disable_irq_nosync(up->port.irq); dev_pm_clear_wake_irq(port->dev); @@ -803,6 +812,7 @@ static void omap_8250_unthrottle(struct uart_port *port) pm_runtime_get_sync(port->dev); + /* Synchronize UART_IER access against the console. */ spin_lock_irqsave(&port->lock, flags); priv->throttled = false; if (up->dma) @@ -953,6 +963,7 @@ static void __dma_rx_complete(void *param) struct dma_tx_state state; unsigned long flags; + /* Synchronize UART_IER access against the console. */ spin_lock_irqsave(&p->port.lock, flags); /* @@ -1227,6 +1238,9 @@ static u16 omap_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir, u16 status static void am654_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir, u16 status) { + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + /* * Queue a new transfer if FIFO has data. */ diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index ecfcdc02eeaf..1321bb42a074 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -539,6 +539,9 @@ EXPORT_SYMBOL_GPL(serial8250_rpm_put); */ static int serial8250_em485_init(struct uart_8250_port *p) { + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&p->port.lock); + if (p->em485) goto deassert_rts; @@ -677,6 +680,8 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) serial8250_rpm_get(p); if (p->capabilities & UART_CAP_SLEEP) { + /* Synchronize UART_IER access against the console. */ + spin_lock_irq(&p->port.lock); if (p->capabilities & UART_CAP_EFR) { lcr = serial_in(p, UART_LCR); efr = serial_in(p, UART_EFR); @@ -690,6 +695,7 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) serial_out(p, UART_EFR, efr); serial_out(p, UART_LCR, lcr); } + spin_unlock_irq(&p->port.lock); } serial8250_rpm_put(p); @@ -697,6 +703,9 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) static void serial8250_clear_IER(struct uart_8250_port *up) { + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + if (up->capabilities & UART_CAP_UUE) serial_out(up, UART_IER, UART_IER_UUE); else @@ -969,6 +978,9 @@ static void autoconfig_16550a(struct uart_8250_port *up) unsigned char status1, status2; unsigned int iersave; + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + up->port.type = PORT_16550A; up->capabilities |= UART_CAP_FIFO; @@ -1152,6 +1164,8 @@ static void autoconfig(struct uart_8250_port *up) /* * We really do need global IRQs disabled here - we're going to * be frobbing the chips IRQ enable register to see if it exists. + * + * Synchronize UART_IER access against the console. */ spin_lock_irqsave(&port->lock, flags); @@ -1324,7 +1338,10 @@ static void autoconfig_irq(struct uart_8250_port *up) /* forget possible initially masked and pending IRQ */ probe_irq_off(probe_irq_on()); save_mcr = serial8250_in_MCR(up); + /* Synchronize UART_IER access against the console. */ + spin_lock_irq(&port->lock); save_ier = serial_in(up, UART_IER); + spin_unlock_irq(&port->lock); serial8250_out_MCR(up, UART_MCR_OUT1 | UART_MCR_OUT2); irqs = probe_irq_on(); @@ -1336,7 +1353,10 @@ static void autoconfig_irq(struct uart_8250_port *up) serial8250_out_MCR(up, UART_MCR_DTR | UART_MCR_RTS | UART_MCR_OUT2); } + /* Synchronize UART_IER access against the console. */ + spin_lock_irq(&port->lock); serial_out(up, UART_IER, UART_IER_ALL_INTR); + spin_unlock_irq(&port->lock); serial_in(up, UART_LSR); serial_in(up, UART_RX); serial_in(up, UART_IIR); @@ -1346,7 +1366,10 @@ static void autoconfig_irq(struct uart_8250_port *up) irq = probe_irq_off(irqs); serial8250_out_MCR(up, save_mcr); + /* Synchronize UART_IER access against the console. */ + spin_lock_irq(&port->lock); serial_out(up, UART_IER, save_ier); + spin_unlock_irq(&port->lock); if (port->flags & UPF_FOURPORT) outb_p(save_ICP, ICP); @@ -1361,6 +1384,9 @@ static void serial8250_stop_rx(struct uart_port *port) { struct uart_8250_port *up = up_to_u8250p(port); + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&port->lock); + serial8250_rpm_get(up); up->ier &= ~(UART_IER_RLSI | UART_IER_RDI); @@ -1380,6 +1406,9 @@ void serial8250_em485_stop_tx(struct uart_8250_port *p) { unsigned char mcr = serial8250_in_MCR(p); + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&p->port.lock); + if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) mcr |= UART_MCR_RTS; else @@ -1429,6 +1458,9 @@ static void __stop_tx_rs485(struct uart_8250_port *p, u64 stop_delay) { struct uart_8250_em485 *em485 = p->em485; + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&p->port.lock); + stop_delay += (u64)p->port.rs485.delay_rts_after_send * NSEC_PER_MSEC; /* @@ -1608,6 +1640,9 @@ static void serial8250_start_tx(struct uart_port *port) struct uart_8250_port *up = up_to_u8250p(port); struct uart_8250_em485 *em485 = up->em485; + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&port->lock); + if (!port->x_char && uart_circ_empty(&port->state->xmit)) return; @@ -1635,6 +1670,9 @@ static void serial8250_disable_ms(struct uart_port *port) { struct uart_8250_port *up = up_to_u8250p(port); + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&port->lock); + /* no MSR capabilities */ if (up->bugs & UART_BUG_NOMSR) return; @@ -1649,6 +1687,9 @@ static void serial8250_enable_ms(struct uart_port *port) { struct uart_8250_port *up = up_to_u8250p(port); + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&port->lock); + /* no MSR capabilities */ if (up->bugs & UART_BUG_NOMSR) return; @@ -2105,6 +2146,14 @@ static void serial8250_put_poll_char(struct uart_port *port, unsigned int ier; struct uart_8250_port *up = up_to_u8250p(port); + /* + * Normally the port is locked to synchronize UART_IER access + * against the console. However, this function is only used by + * KDB/KGDB, where it may not be possible to acquire the port + * lock because all other CPUs are quiesced. The quiescence + * should allow safe lockless usage here. + */ + serial8250_rpm_get(up); /* * First save the IER then disable the interrupts @@ -2440,6 +2489,8 @@ void serial8250_do_shutdown(struct uart_port *port) serial8250_rpm_get(up); /* * Disable interrupts from this port + * + * Synchronize UART_IER access against the console. */ spin_lock_irqsave(&port->lock, flags); up->ier = 0; @@ -2739,6 +2790,8 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios, /* * Ok, we're now changing the port state. Do it with * interrupts disabled. + * + * Synchronize UART_IER access against the console. */ serial8250_rpm_get(up); spin_lock_irqsave(&port->lock, flags); |