* [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR
@ 2025-05-28 6:26 Yunhui Cui
2025-05-28 6:26 ` [PATCH v7 2/4] serial: 8250: avoid potential PSLVERR issue Yunhui Cui
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Yunhui Cui @ 2025-05-28 6:26 UTC (permalink / raw)
To: arnd, andriy.shevchenko, benjamin.larsson, cuiyunhui, gregkh,
heikki.krogerus, ilpo.jarvinen, jirislaby, jkeeping, john.ogness,
linux-kernel, linux-serial, markus.mayer, matt.porter, namcao,
paulmck, pmladek, schnelle, sunilvl, tim.kryger
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>
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v7 2/4] serial: 8250: avoid potential PSLVERR issue
2025-05-28 6:26 [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR Yunhui Cui
@ 2025-05-28 6:26 ` Yunhui Cui
2025-05-28 6:26 ` [PATCH v7 3/4] serial: 8250_dw: assert port->lock is held in dw8250_force_idle() Yunhui Cui
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Yunhui Cui @ 2025-05-28 6:26 UTC (permalink / raw)
To: arnd, andriy.shevchenko, benjamin.larsson, cuiyunhui, gregkh,
heikki.krogerus, ilpo.jarvinen, jirislaby, jkeeping, john.ogness,
linux-kernel, linux-serial, markus.mayer, matt.porter, namcao,
paulmck, pmladek, schnelle, sunilvl, tim.kryger
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);
--
2.39.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v7 3/4] serial: 8250_dw: assert port->lock is held in dw8250_force_idle()
2025-05-28 6:26 [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR Yunhui Cui
2025-05-28 6:26 ` [PATCH v7 2/4] serial: 8250: avoid potential PSLVERR issue Yunhui Cui
@ 2025-05-28 6:26 ` Yunhui Cui
2025-05-28 6:26 ` [PATCH v7 4/4] serial: 8250_dw: fix PSLVERR on RX_TIMEOUT Yunhui Cui
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Yunhui Cui @ 2025-05-28 6:26 UTC (permalink / raw)
To: arnd, andriy.shevchenko, benjamin.larsson, cuiyunhui, gregkh,
heikki.krogerus, ilpo.jarvinen, jirislaby, jkeeping, john.ogness,
linux-kernel, linux-serial, markus.mayer, matt.porter, namcao,
paulmck, pmladek, schnelle, sunilvl, tim.kryger
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
--
2.39.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v7 4/4] serial: 8250_dw: fix PSLVERR on RX_TIMEOUT
2025-05-28 6:26 [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR Yunhui Cui
2025-05-28 6:26 ` [PATCH v7 2/4] serial: 8250: avoid potential PSLVERR issue Yunhui Cui
2025-05-28 6:26 ` [PATCH v7 3/4] serial: 8250_dw: assert port->lock is held in dw8250_force_idle() Yunhui Cui
@ 2025-05-28 6:26 ` Yunhui Cui
2025-06-06 10:41 ` Greg KH
2025-06-03 1:54 ` [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR yunhui cui
2025-06-06 10:40 ` Greg KH
4 siblings, 1 reply; 9+ messages in thread
From: Yunhui Cui @ 2025-05-28 6:26 UTC (permalink / raw)
To: arnd, andriy.shevchenko, benjamin.larsson, cuiyunhui, gregkh,
heikki.krogerus, ilpo.jarvinen, jirislaby, jkeeping, john.ogness,
linux-kernel, linux-serial, markus.mayer, matt.porter, namcao,
paulmck, pmladek, schnelle, sunilvl, tim.kryger
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>
---
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);
}
--
2.39.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR
2025-05-28 6:26 [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR Yunhui Cui
` (2 preceding siblings ...)
2025-05-28 6:26 ` [PATCH v7 4/4] serial: 8250_dw: fix PSLVERR on RX_TIMEOUT Yunhui Cui
@ 2025-06-03 1:54 ` yunhui cui
2025-06-03 4:21 ` Greg KH
2025-06-06 10:40 ` Greg KH
4 siblings, 1 reply; 9+ messages in thread
From: yunhui cui @ 2025-06-03 1:54 UTC (permalink / raw)
To: arnd, andriy.shevchenko, benjamin.larsson, cuiyunhui, gregkh,
heikki.krogerus, ilpo.jarvinen, jirislaby, jkeeping, john.ogness,
linux-kernel, linux-serial, markus.mayer, matt.porter, namcao,
paulmck, pmladek, schnelle, sunilvl, tim.kryger
Hi All,
Gentle ping. Any comments on this patchset?
On Wed, May 28, 2025 at 2:26 PM Yunhui Cui <cuiyunhui@bytedance.com> 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>
> ---
> 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
>
Thanks,
Yunhui
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR
2025-06-03 1:54 ` [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR yunhui cui
@ 2025-06-03 4:21 ` Greg KH
0 siblings, 0 replies; 9+ messages in thread
From: Greg KH @ 2025-06-03 4:21 UTC (permalink / raw)
To: yunhui cui
Cc: arnd, andriy.shevchenko, benjamin.larsson, heikki.krogerus,
ilpo.jarvinen, jirislaby, jkeeping, john.ogness, linux-kernel,
linux-serial, markus.mayer, matt.porter, namcao, paulmck, pmladek,
schnelle, sunilvl, tim.kryger
On Tue, Jun 03, 2025 at 09:54:29AM +0800, yunhui cui wrote:
> Hi All,
>
> Gentle ping. Any comments on this patchset?
My bot responded to you when you sent it a few days ago. Please see
what it said then as to what is going on at the moment.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR
2025-05-28 6:26 [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR Yunhui Cui
` (3 preceding siblings ...)
2025-06-03 1:54 ` [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR yunhui cui
@ 2025-06-06 10:40 ` Greg KH
2025-06-09 7:02 ` [External] " yunhui cui
4 siblings, 1 reply; 9+ messages in thread
From: Greg KH @ 2025-06-06 10:40 UTC (permalink / raw)
To: Yunhui Cui
Cc: arnd, andriy.shevchenko, benjamin.larsson, heikki.krogerus,
ilpo.jarvinen, jirislaby, jkeeping, john.ogness, linux-kernel,
linux-serial, markus.mayer, matt.porter, namcao, paulmck, pmladek,
schnelle, sunilvl, tim.kryger
On Wed, May 28, 2025 at 02:26:06PM +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>
> ---
> 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:
- You have marked a patch with a "Fixes:" tag for a commit that is in an
older released kernel, yet you do not have a cc: stable line in the
signed-off-by area at all, which means that the patch will not be
applied to any older kernel releases. To properly fix this, please
follow the documented rules in the
Documentation/process/stable-kernel-rules.rst file for how to resolve
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v7 4/4] serial: 8250_dw: fix PSLVERR on RX_TIMEOUT
2025-05-28 6:26 ` [PATCH v7 4/4] serial: 8250_dw: fix PSLVERR on RX_TIMEOUT Yunhui Cui
@ 2025-06-06 10:41 ` Greg KH
0 siblings, 0 replies; 9+ messages in thread
From: Greg KH @ 2025-06-06 10:41 UTC (permalink / raw)
To: Yunhui Cui
Cc: arnd, andriy.shevchenko, benjamin.larsson, heikki.krogerus,
ilpo.jarvinen, jirislaby, jkeeping, john.ogness, linux-kernel,
linux-serial, markus.mayer, matt.porter, namcao, paulmck, pmladek,
schnelle, sunilvl, tim.kryger
On Wed, May 28, 2025 at 02:26:09PM +0800, Yunhui Cui wrote:
> 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>
> ---
> 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);
> }
>
> --
> 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:
- You have marked a patch with a "Fixes:" tag for a commit that is in an
older released kernel, yet you do not have a cc: stable line in the
signed-off-by area at all, which means that the patch will not be
applied to any older kernel releases. To properly fix this, please
follow the documented rules in the
Documentation/process/stable-kernel-rules.rst file for how to resolve
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [External] Re: [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR
2025-06-06 10:40 ` Greg KH
@ 2025-06-09 7:02 ` yunhui cui
0 siblings, 0 replies; 9+ messages in thread
From: yunhui cui @ 2025-06-09 7:02 UTC (permalink / raw)
To: Greg KH
Cc: arnd, andriy.shevchenko, benjamin.larsson, heikki.krogerus,
ilpo.jarvinen, jirislaby, jkeeping, john.ogness, linux-kernel,
linux-serial, markus.mayer, matt.porter, namcao, paulmck, pmladek,
schnelle, sunilvl, tim.kryger, stable
Hi,
On Fri, Jun 6, 2025 at 6:40 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, May 28, 2025 at 02:26:06PM +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>
> > ---
> > 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:
>
> - You have marked a patch with a "Fixes:" tag for a commit that is in an
> older released kernel, yet you do not have a cc: stable line in the
> signed-off-by area at all, which means that the patch will not be
> applied to any older kernel releases. To properly fix this, please
> follow the documented rules in the
> Documentation/process/stable-kernel-rules.rst file for how to resolve
> this.
Okay, update under v8.
>
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-06-09 7:02 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-28 6:26 [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR Yunhui Cui
2025-05-28 6:26 ` [PATCH v7 2/4] serial: 8250: avoid potential PSLVERR issue Yunhui Cui
2025-05-28 6:26 ` [PATCH v7 3/4] serial: 8250_dw: assert port->lock is held in dw8250_force_idle() Yunhui Cui
2025-05-28 6:26 ` [PATCH v7 4/4] serial: 8250_dw: fix PSLVERR on RX_TIMEOUT Yunhui Cui
2025-06-06 10:41 ` Greg KH
2025-06-03 1:54 ` [PATCH v7 1/4] serial: 8250: fix panic due to PSLVERR yunhui cui
2025-06-03 4:21 ` Greg KH
2025-06-06 10:40 ` Greg KH
2025-06-09 7:02 ` [External] " yunhui cui
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).