Commit d0b309a5 authored by John Ogness's avatar John Ogness Committed by Greg Kroah-Hartman
Browse files

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: default avatarJohn Ogness <john.ogness@linutronix.de>
Link: https://lore.kernel.org/r/20230525093159.223817-9-john.ogness@linutronix.de


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 25614735

drivers/tty/serial/8250/8250.h

+6 −0
Original line number Diff line number Diff line
@@ -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;

drivers/tty/serial/8250/8250_aspeed_vuart.c

+3 −0
Original line number Diff line number Diff line
@@ -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;

drivers/tty/serial/8250/8250_mtk.c

+9 −0
Original line number Diff line number Diff line
@@ -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);

drivers/tty/serial/8250/8250_omap.c

+14 −0
Original line number Diff line number Diff line
@@ -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.

	 */

drivers/tty/serial/8250/8250_port.c

+53 −0
Original line number Diff line number Diff line
@@ -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);