On our switches with 16550 UART, which has 16 byte FIFOs, it is
very easy to into input buffer overrun. If a printk (of atleast
the size of FIFO) to console happens at the same time our automated
test harness sends a command over serial port. We have seen this
on both Intel and AMD based motherboards that has this UART integrated
in it. We run our switches at 9600 baud which is the industry
standard default speed for console but the issue can be seen even at
higher baudrate.
NOTE that while kernel is writing to console from a SysRq handler,
UART could be buffering input bytes and that can lead to buffer
overrun; this patch does not address that use-case.
Below patch is under new config option SERIAL_8250_RCV_WHILE_XMIT
and default is set to 'n'.
Patch is tested on our switch running 4.0 kernel with and without
enabling SERIAL_8250_RCV_WHILE_XMIT.
Signed-off-by: Prasad Koya <prasad@xxxxxxxxxx>
---
drivers/tty/serial/8250/8250_core.c | 155 ++++++++++++++++++++++--------------
drivers/tty/serial/8250/Kconfig | 11 +++
2 files changed, 107 insertions(+), 59 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 422ebea..ffc1ee3 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1440,6 +1440,69 @@ static void serial8250_enable_ms(struct uart_port *port)
serial8250_rpm_put(up);
}
+static void
+serial8250_rx_one_char(struct uart_8250_port *up, unsigned char lsr)
+{
+ struct uart_port *port = &up->port;
+ unsigned char ch;
+ char flag;
+
+ if (likely(lsr & UART_LSR_DR))
+ ch = serial_in(up, UART_RX);
+ else
+ /*
+ * Intel 82571 has a Serial Over Lan device that will
+ * set UART_LSR_BI without setting UART_LSR_DR when
+ * it receives a break. To avoid reading from the
+ * receive buffer without UART_LSR_DR bit set, we
+ * just force the read character to be 0
+ */
+ ch = 0;
+
+ flag = TTY_NORMAL;
+ port->icount.rx++;
+
+ lsr |= up->lsr_saved_flags;
+ up->lsr_saved_flags = 0;
+
+ if (unlikely(lsr & UART_LSR_BRK_ERROR_BITS)) {
+ if (lsr & UART_LSR_BI) {
+ lsr &= ~(UART_LSR_FE | UART_LSR_PE);
+ port->icount.brk++;
+ /*
+ * We do the SysRQ and SAK checking
+ * here because otherwise the break
+ * may get masked by ignore_status_mask
+ * or read_status_mask.
+ */
+ if (uart_handle_break(port))
+ return;
+ } else if (lsr & UART_LSR_PE)
+ port->icount.parity++;
+ else if (lsr & UART_LSR_FE)
+ port->icount.frame++;
+ if (lsr & UART_LSR_OE)
+ port->icount.overrun++;
+
+ /*
+ * Mask off conditions which should be ignored.
+ */
+ lsr &= port->read_status_mask;
+
+ if (lsr & UART_LSR_BI) {
+ DEBUG_INTR("handling break....");
+ flag = TTY_BREAK;
+ } else if (lsr & UART_LSR_PE)
+ flag = TTY_PARITY;
+ else if (lsr & UART_LSR_FE)
+ flag = TTY_FRAME;
+ }
+ if (uart_handle_sysrq_char(port, ch))
+ return;
+
+ uart_insert_char(port, lsr, UART_LSR_OE, ch, flag);
+}
+
/*
* serial8250_rx_chars: processes according to the passed in LSR
* value, and returns the remaining LSR bits not handled
@@ -1449,67 +1512,10 @@ unsigned char
serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr)
{
struct uart_port *port = &up->port;
- unsigned char ch;
int max_count = 256;
- char flag;
do {
- if (likely(lsr & UART_LSR_DR))
- ch = serial_in(up, UART_RX);
- else
- /*
- * Intel 82571 has a Serial Over Lan device that will
- * set UART_LSR_BI without setting UART_LSR_DR when
- * it receives a break. To avoid reading from the
- * receive buffer without UART_LSR_DR bit set, we
- * just force the read character to be 0
- */
- ch = 0;
-
- flag = TTY_NORMAL;
- port->icount.rx++;
-
- lsr |= up->lsr_saved_flags;
- up->lsr_saved_flags = 0;
-
- if (unlikely(lsr & UART_LSR_BRK_ERROR_BITS)) {
- if (lsr & UART_LSR_BI) {
- lsr &= ~(UART_LSR_FE | UART_LSR_PE);
- port->icount.brk++;
- /*
- * We do the SysRQ and SAK checking
- * here because otherwise the break
- * may get masked by ignore_status_mask
- * or read_status_mask.
- */
- if (uart_handle_break(port))
- goto ignore_char;
- } else if (lsr & UART_LSR_PE)
- port->icount.parity++;
- else if (lsr & UART_LSR_FE)
- port->icount.frame++;
- if (lsr & UART_LSR_OE)
- port->icount.overrun++;
-
- /*
- * Mask off conditions which should be ignored.
- */
- lsr &= port->read_status_mask;
-
- if (lsr & UART_LSR_BI) {
- DEBUG_INTR("handling break....");
- flag = TTY_BREAK;
- } else if (lsr & UART_LSR_PE)
- flag = TTY_PARITY;
- else if (lsr & UART_LSR_FE)
- flag = TTY_FRAME;
- }
- if (uart_handle_sysrq_char(port, ch))
- goto ignore_char;
-
- uart_insert_char(port, lsr, UART_LSR_OE, ch, flag);
-
-ignore_char:
+ serial8250_rx_one_char(up, lsr);
lsr = serial_in(up, UART_LSR);
} while ((lsr & (UART_LSR_DR | UART_LSR_BI)) && (--max_count > 0));
spin_unlock(&port->lock);
@@ -2021,13 +2027,37 @@ static void serial8250_break_ctl(struct uart_port *port, int break_state)
serial8250_rpm_put(up);
}
+#ifdef CONFIG_SERIAL_8250_RCV_WHILE_XMIT
+/*
+ * To avoid buffer overrun, pick up bytes from recv fifo while waiting
+ * for transmit register to be empty
+ */
+static void rx_while_wait_for_xmitr(struct uart_8250_port *up, int lsr)
+{
+ struct uart_port *port = &up->port;
+ /* Do not process recv FIFO while handling SYSRQ */
+ if ((lsr & UART_LSR_DR) && !port->sysrq && !oops_in_progress &&
+ !(lsr & UART_LSR_BRK_ERROR_BITS))
+ serial8250_rx_one_char(up, lsr);
+}
+#else
+#define rx_while_wait_for_xmitr(up, lsr) do { } while (0)
+#endif
+
/*
* Wait for transmitter & holding register to empty
*/
static void wait_for_xmitr(struct uart_8250_port *up, int bits)
{
unsigned int status, tmout = 10000;
+ int rx = 0;
+#ifdef CONFIG_SERIAL_8250_RCV_WHILE_XMIT
+ if (bits & UART_LSR_DR) {
+ rx = 1;
+ bits &= ~UART_LSR_DR;
+ }
+#endif
/* Wait up to 10ms for the character(s) to be sent. */
for (;;) {
status = serial_in(up, UART_LSR);
@@ -2038,6 +2068,8 @@ static void wait_for_xmitr(struct uart_8250_port *up, int bits)
break;
if (--tmout == 0)
break;
+ if (unlikely(rx))
+ rx_while_wait_for_xmitr(up, status);
udelay(1);
}
@@ -3319,8 +3351,13 @@ serial8250_register_ports(struct uart_driver *drv, struct device *dev)
static void serial8250_console_putchar(struct uart_port *port, int ch)
{
struct uart_8250_port *up = up_to_u8250p(port);
+ int bits = UART_LSR_THRE;
- wait_for_xmitr(up, UART_LSR_THRE);
+#ifdef CONFIG_SERIAL_8250_RCV_WHILE_XMIT
+ /* While waiting for THR to be empty, read from fifo if LSR_DR is set */
+ bits |= UART_LSR_DR;
+#endif
+ wait_for_xmitr(up, bits);
serial_port_out(port, UART_TX, ch);
}
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index c350703..9715ccd 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -342,3 +342,14 @@ config SERIAL_8250_MT6577
help
If you have a Mediatek based board and want to use the
serial port, say Y to this option. If unsure, say N.
+
+config SERIAL_8250_RCV_WHILE_XMIT
+ bool "Read from receive FIFO while transmitting"
+ depends on SERIAL_8250_CONSOLE
+ default n
+ help
+ If you say 'y' here, 8250 driver will check and read from input FIFO during
+ transmission. This can avoid input buffer overrun by reading from receive
+ FIFO while waiting for transmitter to be empty. Note that this has no effect
+ on sysrq handling i.e., if transmitter is busy during sysrq handling, buffer
+ overrun is still possible.
--
1.8.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
