Re: [PATCH] serial: 8250: fix panic due to PSLVERR

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, Apr 03, 2025 at 05:03:36PM +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,

serial8250_do_startup()

> 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 dont_test_tx_en label:
> ...
> serial_port_in(port, UART_RX);
> This satisfies the PSLVERR trigger condition.
> 
> Because another CPU(e.g., using printk) is accessing the UART (UART

printk()

> is busy), the current CPU fails the check (value & ~UART_LCR_SPAR) ==
> (lcr & ~UART_LCR_SPAR), causing it to enter dw8250_force_idle().
> 
> To resolve this issue, relevant serial_port_out operations should be

serial_port_out()

> placed in a critical section, and UART_RX data should only be read
> when the UART_LSR DR bit is set.

The last one is made in the common code, are you sure that all supported UARTs
will be okay with such a change?

> Panic message:

Please, read this
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages
and act accordingly.

> [    0.442336] Oops - unknown exception [#1]
> [    0.442337] Modules linked in:
> [    0.442339] CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Tainted: G        W          6.12.13-00102-gf1f43e345877 #1

Is it still reproducible on v6.14 (and soon v6.15-rc1)?

> [    0.442342] Tainted: [W]=WARN
> [    0.442343] epc : dw8250_serial_in32+0x1e/0x4a
> [    0.442351]  ra : serial8250_do_startup+0x2c8/0x88e
> [    0.442354] epc : ffffffff8064efca ra : ffffffff8064af28 sp : ffff8f8000103990
> [    0.442355]  gp : ffffffff815bad28 tp : ffffaf807e36d400 t0 : ffffaf80804cf080
> [    0.442356]  t1 : 0000000000000001 t2 : 0000000000000000 s0 : ffff8f80001039a0
> [    0.442358]  s1 : ffffffff81626fc0 a0 : ffffffff81626fc0 a1 : 0000000000000000
> [    0.442359]  a2 : 0000000000000000 a3 : 0000000000000000 a4 : ffffffff81626fc0
> [    0.442360]  a5 : ffff8f800012d900 a6 : 000000000000000f a7 : 000000000fc648c1
> [    0.442361]  s2 : 0000000000000000 s3 : 0000000200000022 s4 : 0000000000000000
> [    0.442362]  s5 : ffffffff81626fc0 s6 : ffffaf8085227000 s7 : ffffffff81073c58
> [    0.442363]  s8 : 0000000000500000 s9 : ffffaf80851a5a60 s10: ffffaf80851a5a60
> [    0.442365]  s11: ffffffff80e85980 t3 : ffffaf807e324600 t4 : 0000000000000002
> [    0.442365]  t5 : 0000000000000003 t6 : ffffaf80804cf072
> [    0.442366] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000013
> [    0.442368] [<ffffffff8064efca>] dw8250_serial_in32+0x1e/0x4a
> [    0.442371] [<ffffffff8064af28>] serial8250_do_startup+0x2c8/0x88e
> [    0.442373] [<ffffffff8064b514>] serial8250_startup+0x26/0x2e
> [    0.442375] [<ffffffff806428a2>] uart_startup+0x13a/0x308
> [    0.442377] [<ffffffff80642aa4>] uart_port_activate+0x34/0x50
> [    0.442378] [<ffffffff8062ab6a>] tty_port_open+0xb4/0x110
> [    0.442383] [<ffffffff8063f548>] uart_open+0x22/0x36
> [    0.442389] [<ffffffff806234b4>] tty_open+0x1be/0x5e6
> [    0.442396] [<ffffffff802f2d52>] chrdev_open+0x10a/0x2a8
> [    0.442400] [<ffffffff802e7ab6>] do_dentry_open+0xf6/0x34e
> [    0.442405] [<ffffffff802e9456>] vfs_open+0x2a/0xb4
> [    0.442408] [<ffffffff80300124>] path_openat+0x676/0xf36
> [    0.442410] [<ffffffff80300a58>] do_filp_open+0x74/0xfa
> [    0.442412] [<ffffffff802e9900>] file_open_name+0x84/0x144
> [    0.442414] [<ffffffff802e99f6>] filp_open+0x36/0x54
> [    0.442416] [<ffffffff80a01232>] console_on_rootfs+0x26/0x70
> [    0.442420] [<ffffffff80a0154e>] kernel_init_freeable+0x2d2/0x30e
> [    0.442422] [<ffffffff8099c730>] kernel_init+0x2a/0x15e
> [    0.442427] [<ffffffff809a7666>] ret_from_fork+0xe/0x1c
> [    0.442430] Code: e022 e406 0800 4683 0c15 691c 872a 96bb 00d5 97b6 (439c) 851b
> [    0.442432] ---[ end trace 0000000000000000 ]---
> [    0.442434] Kernel panic - not syncing: Fatal exception in interrupt
> [    0.442435] SMP: stopping secondary CPUs
> [    0.451111] ---[ end Kernel panic - not syncing: Fatal exception in interrupt ]---

Fixes tag?
Cc to stable@?

...

>  	/*
>  	 * Now, initialize the UART
>  	 */

+ Blank line.

> +	uart_port_lock_irqsave(port, &flags);
>  	serial_port_out(port, UART_LCR, UART_LCR_WLEN8);
>  
> -	uart_port_lock_irqsave(port, &flags);
>  	if (up->port.flags & UPF_FOURPORT) {
>  		if (!up->port.irq)
>  			up->port.mctrl |= TIOCM_OUT1;

-- 
With Best Regards,
Andy Shevchenko