When the PSLVERR_RESP_EN parameter is set to 1, the device generates an error response if an attempt is made to read an empty RBR (Receive Buffer Register) while the FIFO is enabled.
In serial8250_do_startup(), calling serial_port_out(port, UART_LCR, UART_LCR_WLEN8) triggers dw8250_check_lcr(), which invokes dw8250_force_idle() and serial8250_clear_and_reinit_fifos(). The latter function enables the FIFO via serial_out(p, UART_FCR, p->fcr). Execution proceeds to the serial_port_in(port, UART_RX). This satisfies the PSLVERR trigger condition.
When another CPU (e.g., using printk()) is accessing the UART (UART is busy), the current CPU fails the check (value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR) in dw8250_check_lcr(), causing it to enter dw8250_force_idle().
Put serial_port_out(port, UART_LCR, UART_LCR_WLEN8) under the port->lock to fix this issue.
Panic backtrace: [ 0.442336] Oops - unknown exception [#1] [ 0.442343] epc : dw8250_serial_in32+0x1e/0x4a [ 0.442351] ra : serial8250_do_startup+0x2c8/0x88e ... [ 0.442416] console_on_rootfs+0x26/0x70
Fixes: c49436b657d0 ("serial: 8250_dw: Improve unwritable LCR workaround") Link: https://lore.kernel.org/all/84cydt5peu.fsf@jogness.linutronix.de/T/ Signed-off-by: Yunhui Cui cuiyunhui@bytedance.com Cc: stable@vger.kernel.org --- drivers/tty/serial/8250/8250_port.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 6d7b8c4667c9c..07fe818dffa34 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -2376,9 +2376,10 @@ int serial8250_do_startup(struct uart_port *port) /* * Now, initialize the UART */ - serial_port_out(port, UART_LCR, UART_LCR_WLEN8);
uart_port_lock_irqsave(port, &flags); + serial_port_out(port, UART_LCR, UART_LCR_WLEN8); + if (up->port.flags & UPF_FOURPORT) { if (!up->port.irq) up->port.mctrl |= TIOCM_OUT1;
When the PSLVERR_RESP_EN parameter is set to 1, reading UART_RX while the FIFO is enabled and UART_LSR_DR is not set will generate a PSLVERR error.
Failure to check the UART_LSR_DR before reading UART_RX, or the non- atomic nature of clearing the FIFO and reading UART_RX, poses potential risks that could lead to PSLVERR.
PSLVERR is addressed through two methods. One is to introduce serial8250_discard_data() to check whether UART_LSR_DR is set before reading UART_RX, thus solving the PSLVERR issue when the FIFO is enabled. The other is to place FIFO clearing and reading of UART_RX under port->lock.
Signed-off-by: Yunhui Cui cuiyunhui@bytedance.com --- drivers/tty/serial/8250/8250.h | 13 +++++++++++++ drivers/tty/serial/8250/8250_port.c | 26 +++++++++++++++----------- 2 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h index 18530c31a5981..b3fb8a550db35 100644 --- a/drivers/tty/serial/8250/8250.h +++ b/drivers/tty/serial/8250/8250.h @@ -162,6 +162,19 @@ static inline u16 serial_lsr_in(struct uart_8250_port *up) return lsr; }
+/* + * To avoid PSLVERR, check UART_LSR_DR in UART_LSR before + * reading UART_RX. + */ +static inline void serial8250_discard_data(struct uart_8250_port *up) +{ + u16 lsr; + + lsr = serial_in(up, UART_LSR); + if (lsr & UART_LSR_DR) + serial_in(up, UART_RX); +} + /* * For the 16C950 */ diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 07fe818dffa34..0560df9b064f9 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -764,8 +764,6 @@ static void disable_rsa(struct uart_8250_port *up)
if (up->port.type == PORT_RSA && up->port.uartclk == SERIAL_RSA_BAUD_BASE * 16) { - uart_port_lock_irq(&up->port); - mode = serial_in(up, UART_RSA_MSR); result = !(mode & UART_RSA_MSR_FIFO);
@@ -777,7 +775,6 @@ static void disable_rsa(struct uart_8250_port *up)
if (result) up->port.uartclk = SERIAL_RSA_BAUD_BASE_LO * 16; - uart_port_unlock_irq(&up->port); } } #endif /* CONFIG_SERIAL_8250_RSA */ @@ -1353,9 +1350,8 @@ static void autoconfig_irq(struct uart_8250_port *up) /* Synchronize UART_IER access against the console. */ uart_port_lock_irq(port); serial_out(up, UART_IER, UART_IER_ALL_INTR); + serial8250_discard_data(up); uart_port_unlock_irq(port); - serial_in(up, UART_LSR); - serial_in(up, UART_RX); serial_in(up, UART_IIR); serial_in(up, UART_MSR); serial_out(up, UART_TX, 0xFF); @@ -2260,13 +2256,20 @@ int serial8250_do_startup(struct uart_port *port) * Clear the FIFO buffers and disable them. * (they will be reenabled in set_termios()) */ + uart_port_lock_irqsave(port, &flags); serial8250_clear_fifos(up);
/* - * Clear the interrupt registers. + * Read UART_RX to clear interrupts (e.g., Character Timeout). + * To prevent PSLVERR, we can either disable the FIFO before reading + * UART_RX or read UART_RX only when UART_LSR_DR is set while the FIFO + * remains enabled. If using the latter approach to avoid PSLVERR, it + * creates a contradiction with the interrupt-clearing (see the + * rx_timeout handling in dw8250_handle_irq()). */ serial_port_in(port, UART_LSR); serial_port_in(port, UART_RX); + uart_port_unlock_irqrestore(port, flags); serial_port_in(port, UART_IIR); serial_port_in(port, UART_MSR);
@@ -2423,15 +2426,13 @@ int serial8250_do_startup(struct uart_port *port) } }
- uart_port_unlock_irqrestore(port, flags); - /* * Clear the interrupt registers again for luck, and clear the * saved flags to avoid getting false values from polling * routines or the previous session. */ - serial_port_in(port, UART_LSR); - serial_port_in(port, UART_RX); + serial8250_discard_data(up); + uart_port_unlock_irqrestore(port, flags); serial_port_in(port, UART_IIR); serial_port_in(port, UART_MSR); up->lsr_saved_flags = 0; @@ -2513,7 +2514,6 @@ void serial8250_do_shutdown(struct uart_port *port) port->mctrl &= ~TIOCM_OUT2;
serial8250_set_mctrl(port, port->mctrl); - uart_port_unlock_irqrestore(port, flags);
/* * Disable break condition and FIFOs @@ -2532,8 +2532,12 @@ void serial8250_do_shutdown(struct uart_port *port) /* * Read data port to reset things, and then unlink from * the IRQ chain. + * + * Since reading UART_RX clears interrupts, doing so with + * FIFO disabled won't trigger PSLVERR. */ serial_port_in(port, UART_RX); + uart_port_unlock_irqrestore(port, flags); serial8250_rpm_put(up);
up->ops->release_irq(up);
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [PATCH v8 2/4] serial: 8250: avoid potential PSLVERR issue Link: https://lore.kernel.org/stable/20250609074348.54899-2-cuiyunhui%40bytedance....
Hi,
On Mon, Jun 9, 2025 at 3:45 PM kernel test robot lkp@intel.com wrote:
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree.
This is an improvement and does not require adding Cc: stable@vger.kernel.org.
Subject: [PATCH v8 2/4] serial: 8250: avoid potential PSLVERR issue Link: https://lore.kernel.org/stable/20250609074348.54899-2-cuiyunhui%40bytedance....
-- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Thanks, Yunhui
Reading UART_RX and checking whether UART_LSR_DR is set should be atomic. Ensure the caller of dw8250_force_idle() holds port->lock.
Signed-off-by: Yunhui Cui cuiyunhui@bytedance.com --- drivers/tty/serial/8250/8250_dw.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index 1902f29444a1c..8b0018fadccea 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -13,6 +13,7 @@ #include <linux/delay.h> #include <linux/device.h> #include <linux/io.h> +#include <linux/lockdep.h> #include <linux/mod_devicetable.h> #include <linux/module.h> #include <linux/notifier.h> @@ -117,6 +118,9 @@ static void dw8250_force_idle(struct uart_port *p) struct uart_8250_port *up = up_to_u8250p(p); unsigned int lsr;
+ /* Reading UART_LSR and UART_RX should be atomic. */ + lockdep_assert_held_once(&p->lock); + /* * The following call currently performs serial_out() * against the FCR register. Because it differs to LCR
The DW UART may trigger the RX_TIMEOUT interrupt without data present and remain stuck in this state indefinitely. The dw8250_handle_irq() function detects this condition by checking if the UART_LSR_DR bit is not set when RX_TIMEOUT occurs. When detected, it performs a "dummy read" to recover the DW UART from this state.
When the PSLVERR_RESP_EN parameter is set to 1, reading the UART_RX while the FIFO is enabled and UART_LSR_DR is not set will generate a PSLVERR error, which may lead to a system panic. There are two methods to prevent PSLVERR: one is to check if UART_LSR_DR is set before reading UART_RX when the FIFO is enabled, and the other is to read UART_RX when the FIFO is disabled.
Given these two scenarios, the FIFO must be disabled before the "dummy read" operation and re-enabled afterward to maintain normal UART functionality.
Fixes: 424d79183af0 ("serial: 8250_dw: Avoid "too much work" from bogus rx timeout interrupt") Signed-off-by: Yunhui Cui cuiyunhui@bytedance.com Cc: stable@vger.kernel.org --- drivers/tty/serial/8250/8250_dw.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index 8b0018fadccea..686f9117a3339 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -301,9 +301,17 @@ static int dw8250_handle_irq(struct uart_port *p) uart_port_lock_irqsave(p, &flags); status = serial_lsr_in(up);
- if (!(status & (UART_LSR_DR | UART_LSR_BI))) + if (!(status & (UART_LSR_DR | UART_LSR_BI))) { + /* To avoid PSLVERR, disable the FIFO first. */ + if (up->fcr & UART_FCR_ENABLE_FIFO) + serial_out(up, UART_FCR, 0); + serial_port_in(p, UART_RX);
+ if (up->fcr & UART_FCR_ENABLE_FIFO) + serial_out(up, UART_FCR, up->fcr); + } + uart_port_unlock_irqrestore(p, flags); }
On Mon, Jun 09, 2025 at 03:43:45PM +0800, Yunhui Cui wrote:
When the PSLVERR_RESP_EN parameter is set to 1, the device generates an error response if an attempt is made to read an empty RBR (Receive Buffer Register) while the FIFO is enabled.
In serial8250_do_startup(), calling serial_port_out(port, UART_LCR, UART_LCR_WLEN8) triggers dw8250_check_lcr(), which invokes dw8250_force_idle() and serial8250_clear_and_reinit_fifos(). The latter function enables the FIFO via serial_out(p, UART_FCR, p->fcr). Execution proceeds to the serial_port_in(port, UART_RX). This satisfies the PSLVERR trigger condition.
When another CPU (e.g., using printk()) is accessing the UART (UART is busy), the current CPU fails the check (value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR) in dw8250_check_lcr(), causing it to enter dw8250_force_idle().
Put serial_port_out(port, UART_LCR, UART_LCR_WLEN8) under the port->lock to fix this issue.
Panic backtrace: [ 0.442336] Oops - unknown exception [#1] [ 0.442343] epc : dw8250_serial_in32+0x1e/0x4a [ 0.442351] ra : serial8250_do_startup+0x2c8/0x88e ... [ 0.442416] console_on_rootfs+0x26/0x70
Fixes: c49436b657d0 ("serial: 8250_dw: Improve unwritable LCR workaround") Link: https://lore.kernel.org/all/84cydt5peu.fsf@jogness.linutronix.de/T/ Signed-off-by: Yunhui Cui cuiyunhui@bytedance.com Cc: stable@vger.kernel.org
drivers/tty/serial/8250/8250_port.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 6d7b8c4667c9c..07fe818dffa34 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -2376,9 +2376,10 @@ int serial8250_do_startup(struct uart_port *port) /* * Now, initialize the UART */
- serial_port_out(port, UART_LCR, UART_LCR_WLEN8);
uart_port_lock_irqsave(port, &flags);
- serial_port_out(port, UART_LCR, UART_LCR_WLEN8);
- if (up->port.flags & UPF_FOURPORT) { if (!up->port.irq) up->port.mctrl |= TIOCM_OUT1;
-- 2.39.5
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree.
You are receiving this message because of the following common error(s) as indicated below:
- This looks like a new version of a previously submitted patch, but you did not list below the --- line any changes from the previous version. Please read the section entitled "The canonical patch format" in the kernel file, Documentation/process/submitting-patches.rst for what needs to be done here to properly describe this.
If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers.
thanks,
greg k-h's patch email bot
Hi Greg,
On Mon, Jun 9, 2025 at 6:10 PM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jun 09, 2025 at 03:43:45PM +0800, Yunhui Cui wrote:
When the PSLVERR_RESP_EN parameter is set to 1, the device generates an error response if an attempt is made to read an empty RBR (Receive Buffer Register) while the FIFO is enabled.
In serial8250_do_startup(), calling serial_port_out(port, UART_LCR, UART_LCR_WLEN8) triggers dw8250_check_lcr(), which invokes dw8250_force_idle() and serial8250_clear_and_reinit_fifos(). The latter function enables the FIFO via serial_out(p, UART_FCR, p->fcr). Execution proceeds to the serial_port_in(port, UART_RX). This satisfies the PSLVERR trigger condition.
When another CPU (e.g., using printk()) is accessing the UART (UART is busy), the current CPU fails the check (value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR) in dw8250_check_lcr(), causing it to enter dw8250_force_idle().
Put serial_port_out(port, UART_LCR, UART_LCR_WLEN8) under the port->lock to fix this issue.
Panic backtrace: [ 0.442336] Oops - unknown exception [#1] [ 0.442343] epc : dw8250_serial_in32+0x1e/0x4a [ 0.442351] ra : serial8250_do_startup+0x2c8/0x88e ... [ 0.442416] console_on_rootfs+0x26/0x70
Fixes: c49436b657d0 ("serial: 8250_dw: Improve unwritable LCR workaround") Link: https://lore.kernel.org/all/84cydt5peu.fsf@jogness.linutronix.de/T/ Signed-off-by: Yunhui Cui cuiyunhui@bytedance.com Cc: stable@vger.kernel.org
drivers/tty/serial/8250/8250_port.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 6d7b8c4667c9c..07fe818dffa34 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -2376,9 +2376,10 @@ int serial8250_do_startup(struct uart_port *port) /* * Now, initialize the UART */
serial_port_out(port, UART_LCR, UART_LCR_WLEN8); uart_port_lock_irqsave(port, &flags);
serial_port_out(port, UART_LCR, UART_LCR_WLEN8);if (up->port.flags & UPF_FOURPORT) { if (!up->port.irq) up->port.mctrl |= TIOCM_OUT1;-- 2.39.5
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree.
You are receiving this message because of the following common error(s) as indicated below:
- This looks like a new version of a previously submitted patch, but you did not list below the --- line any changes from the previous version. Please read the section entitled "The canonical patch format" in the kernel file, Documentation/process/submitting-patches.rst for what needs to be done here to properly describe this.
Can this issue reported by the bot be ignored?
If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers.
thanks,
greg k-h's patch email bot
Thanks, Yunhui
On Mon, Jun 09, 2025 at 09:18:02PM +0800, yunhui cui wrote:
Hi Greg,
On Mon, Jun 9, 2025 at 6:10 PM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jun 09, 2025 at 03:43:45PM +0800, Yunhui Cui wrote:
When the PSLVERR_RESP_EN parameter is set to 1, the device generates an error response if an attempt is made to read an empty RBR (Receive Buffer Register) while the FIFO is enabled.
In serial8250_do_startup(), calling serial_port_out(port, UART_LCR, UART_LCR_WLEN8) triggers dw8250_check_lcr(), which invokes dw8250_force_idle() and serial8250_clear_and_reinit_fifos(). The latter function enables the FIFO via serial_out(p, UART_FCR, p->fcr). Execution proceeds to the serial_port_in(port, UART_RX). This satisfies the PSLVERR trigger condition.
When another CPU (e.g., using printk()) is accessing the UART (UART is busy), the current CPU fails the check (value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR) in dw8250_check_lcr(), causing it to enter dw8250_force_idle().
Put serial_port_out(port, UART_LCR, UART_LCR_WLEN8) under the port->lock to fix this issue.
Panic backtrace: [ 0.442336] Oops - unknown exception [#1] [ 0.442343] epc : dw8250_serial_in32+0x1e/0x4a [ 0.442351] ra : serial8250_do_startup+0x2c8/0x88e ... [ 0.442416] console_on_rootfs+0x26/0x70
Fixes: c49436b657d0 ("serial: 8250_dw: Improve unwritable LCR workaround") Link: https://lore.kernel.org/all/84cydt5peu.fsf@jogness.linutronix.de/T/ Signed-off-by: Yunhui Cui cuiyunhui@bytedance.com Cc: stable@vger.kernel.org
drivers/tty/serial/8250/8250_port.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 6d7b8c4667c9c..07fe818dffa34 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -2376,9 +2376,10 @@ int serial8250_do_startup(struct uart_port *port) /* * Now, initialize the UART */
serial_port_out(port, UART_LCR, UART_LCR_WLEN8); uart_port_lock_irqsave(port, &flags);
serial_port_out(port, UART_LCR, UART_LCR_WLEN8);if (up->port.flags & UPF_FOURPORT) { if (!up->port.irq) up->port.mctrl |= TIOCM_OUT1;-- 2.39.5
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree.
You are receiving this message because of the following common error(s) as indicated below:
- This looks like a new version of a previously submitted patch, but you did not list below the --- line any changes from the previous version. Please read the section entitled "The canonical patch format" in the kernel file, Documentation/process/submitting-patches.rst for what needs to be done here to properly describe this.
Can this issue reported by the bot be ignored?
No, why? How do we know what changed from previous versions? Otherwise we assume you just ignored previous review comments?
This series took at least 8 tries for some reason, might as well document it, right? :)
thanks,
greg k-h
On Mon, Jun 09, 2025 at 03:44:19PM +0200, Greg KH wrote:
On Mon, Jun 09, 2025 at 09:18:02PM +0800, yunhui cui wrote:
On Mon, Jun 9, 2025 at 6:10 PM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jun 09, 2025 at 03:43:45PM +0800, Yunhui Cui wrote:
...
- This looks like a new version of a previously submitted patch, but you did not list below the --- line any changes from the previous version. Please read the section entitled "The canonical patch format" in the kernel file, Documentation/process/submitting-patches.rst for what needs to be done here to properly describe this.
Can this issue reported by the bot be ignored?
No, why? How do we know what changed from previous versions? Otherwise we assume you just ignored previous review comments?
This series took at least 8 tries for some reason, might as well document it, right? :)
+ many to what Greg said. The main problem you have is the absence of cover letter, where it's naturally to put changelog for the series.
linux-stable-mirror@lists.linaro.org
-
Andy Shevchenko -
Greg KH -
kernel test robot -
Yunhui Cui -
yunhui cui