Skip to content

Commit a99a241

Browse files
authored
Merge pull request #134 from ukleinek/fix-kernel-lockup
kernel: Add a patch to fix an NPD when using the UART and a stuck irq
2 parents cb4386e + bc2c396 commit a99a241

20 files changed

+377
-29
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
From: Andy Shevchenko <[email protected]>
2+
Date: Thu, 4 Apr 2024 17:59:26 +0300
3+
Subject: [PATCH] serial: core: Clearing the circular buffer before NULLifying
4+
it
5+
6+
The circular buffer is NULLified in uart_tty_port_shutdown()
7+
under the spin lock. However, the PM or other timer based callbacks
8+
may still trigger after this event without knowning that buffer pointer
9+
is not valid. Since the serial code is a bit inconsistent in checking
10+
the buffer state (some rely on the head-tail positions, some on the
11+
buffer pointer), it's better to have both aligned, i.e. buffer pointer
12+
to be NULL and head-tail possitions to be the same, meaning it's empty.
13+
This will prevent asynchronous calls to dereference NULL pointer as
14+
reported recently in 8250 case:
15+
16+
BUG: kernel NULL pointer dereference, address: 00000cf5
17+
Workqueue: pm pm_runtime_work
18+
EIP: serial8250_tx_chars (drivers/tty/serial/8250/8250_port.c:1809)
19+
...
20+
? serial8250_tx_chars (drivers/tty/serial/8250/8250_port.c:1809)
21+
__start_tx (drivers/tty/serial/8250/8250_port.c:1551)
22+
serial8250_start_tx (drivers/tty/serial/8250/8250_port.c:1654)
23+
serial_port_runtime_suspend (include/linux/serial_core.h:667 drivers/tty/serial/serial_port.c:63)
24+
__rpm_callback (drivers/base/power/runtime.c:393)
25+
? serial_port_remove (drivers/tty/serial/serial_port.c:50)
26+
rpm_suspend (drivers/base/power/runtime.c:447)
27+
28+
The proposed change will prevent ->start_tx() to be called during
29+
suspend on shut down port.
30+
31+
Fixes: 43066e32227e ("serial: port: Don't suspend if the port is still busy")
32+
Cc: stable <[email protected]>
33+
Reported-by: kernel test robot <[email protected]>
34+
Closes: https://lore.kernel.org/oe-lkp/[email protected]
35+
Signed-off-by: Andy Shevchenko <[email protected]>
36+
Link: https://lore.kernel.org/r/[email protected]
37+
Signed-off-by: Greg Kroah-Hartman <[email protected]>
38+
Origin: next-20240410, commit:9cf7ea2eeb745213dc2a04103e426b960e807940
39+
---
40+
drivers/tty/serial/serial_core.c | 1 +
41+
1 file changed, 1 insertion(+)
42+
43+
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
44+
index d6a58a9e072a..15664cda4fcd 100644
45+
--- a/drivers/tty/serial/serial_core.c
46+
+++ b/drivers/tty/serial/serial_core.c
47+
@@ -1788,6 +1788,7 @@ static void uart_tty_port_shutdown(struct tty_port *port)
48+
* Free the transmit buffer.
49+
*/
50+
uart_port_lock_irq(uport);
51+
+ uart_circ_clear(&state->xmit);
52+
buf = state->xmit.buf;
53+
state->xmit.buf = NULL;
54+
uart_port_unlock_irq(uport);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
From: Tony Lindgren <[email protected]>
2+
Date: Thu, 11 Apr 2024 08:58:45 +0300
3+
Subject: [PATCH] serial: core: Fix missing shutdown and startup for serial
4+
base port
5+
6+
We are seeing start_tx being called after port shutdown as noted by Jiri.
7+
This happens because we are missing the startup and shutdown related
8+
functions for the serial base port.
9+
10+
Let's fix the issue by adding startup and shutdown functions for the
11+
serial base port to block tx flushing for the serial base port when the
12+
port is not in use.
13+
14+
Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
15+
Cc: stable <[email protected]>
16+
Reported-by: Jiri Slaby <[email protected]>
17+
Signed-off-by: Tony Lindgren <[email protected]>
18+
Link: https://lore.kernel.org/r/[email protected]
19+
Signed-off-by: Greg Kroah-Hartman <[email protected]>
20+
Origin: next-20240415, commit:1aa4ad4eb695bac1b0a7ba542a16d6833c9c8dd8
21+
---
22+
drivers/tty/serial/serial_base.h | 4 ++++
23+
drivers/tty/serial/serial_core.c | 20 +++++++++++++++++---
24+
drivers/tty/serial/serial_port.c | 34 ++++++++++++++++++++++++++++++++++
25+
3 files changed, 55 insertions(+), 3 deletions(-)
26+
27+
diff --git a/drivers/tty/serial/serial_base.h b/drivers/tty/serial/serial_base.h
28+
index c74c548f0db6..b6c38d2edfd4 100644
29+
--- a/drivers/tty/serial/serial_base.h
30+
+++ b/drivers/tty/serial/serial_base.h
31+
@@ -22,6 +22,7 @@ struct serial_ctrl_device {
32+
struct serial_port_device {
33+
struct device dev;
34+
struct uart_port *port;
35+
+ unsigned int tx_enabled:1;
36+
};
37+
38+
int serial_base_ctrl_init(void);
39+
@@ -30,6 +31,9 @@ void serial_base_ctrl_exit(void);
40+
int serial_base_port_init(void);
41+
void serial_base_port_exit(void);
42+
43+
+void serial_base_port_startup(struct uart_port *port);
44+
+void serial_base_port_shutdown(struct uart_port *port);
45+
+
46+
int serial_base_driver_register(struct device_driver *driver);
47+
void serial_base_driver_unregister(struct device_driver *driver);
48+
49+
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
50+
index 15664cda4fcd..fa9bc6f5035a 100644
51+
--- a/drivers/tty/serial/serial_core.c
52+
+++ b/drivers/tty/serial/serial_core.c
53+
@@ -323,16 +323,26 @@ static int uart_startup(struct tty_struct *tty, struct uart_state *state,
54+
bool init_hw)
55+
{
56+
struct tty_port *port = &state->port;
57+
+ struct uart_port *uport;
58+
int retval;
59+
60+
if (tty_port_initialized(port))
61+
- return 0;
62+
+ goto out_base_port_startup;
63+
64+
retval = uart_port_startup(tty, state, init_hw);
65+
- if (retval)
66+
+ if (retval) {
67+
set_bit(TTY_IO_ERROR, &tty->flags);
68+
+ return retval;
69+
+ }
70+
71+
- return retval;
72+
+out_base_port_startup:
73+
+ uport = uart_port_check(state);
74+
+ if (!uport)
75+
+ return -EIO;
76+
+
77+
+ serial_base_port_startup(uport);
78+
+
79+
+ return 0;
80+
}
81+
82+
/*
83+
@@ -355,6 +365,9 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
84+
if (tty)
85+
set_bit(TTY_IO_ERROR, &tty->flags);
86+
87+
+ if (uport)
88+
+ serial_base_port_shutdown(uport);
89+
+
90+
if (tty_port_initialized(port)) {
91+
tty_port_set_initialized(port, false);
92+
93+
@@ -1775,6 +1788,7 @@ static void uart_tty_port_shutdown(struct tty_port *port)
94+
uport->ops->stop_rx(uport);
95+
uart_port_unlock_irq(uport);
96+
97+
+ serial_base_port_shutdown(uport);
98+
uart_port_shutdown(port);
99+
100+
/*
101+
diff --git a/drivers/tty/serial/serial_port.c b/drivers/tty/serial/serial_port.c
102+
index 72b6f4f326e2..7d51e66ec88b 100644
103+
--- a/drivers/tty/serial/serial_port.c
104+
+++ b/drivers/tty/serial/serial_port.c
105+
@@ -36,8 +36,12 @@ static int serial_port_runtime_resume(struct device *dev)
106+
107+
/* Flush any pending TX for the port */
108+
uart_port_lock_irqsave(port, &flags);
109+
+ if (!port_dev->tx_enabled)
110+
+ goto unlock;
111+
if (__serial_port_busy(port))
112+
port->ops->start_tx(port);
113+
+
114+
+unlock:
115+
uart_port_unlock_irqrestore(port, flags);
116+
117+
out:
118+
@@ -57,6 +61,11 @@ static int serial_port_runtime_suspend(struct device *dev)
119+
return 0;
120+
121+
uart_port_lock_irqsave(port, &flags);
122+
+ if (!port_dev->tx_enabled) {
123+
+ uart_port_unlock_irqrestore(port, flags);
124+
+ return 0;
125+
+ }
126+
+
127+
busy = __serial_port_busy(port);
128+
if (busy)
129+
port->ops->start_tx(port);
130+
@@ -68,6 +77,31 @@ static int serial_port_runtime_suspend(struct device *dev)
131+
return busy ? -EBUSY : 0;
132+
}
133+
134+
+static void serial_base_port_set_tx(struct uart_port *port,
135+
+ struct serial_port_device *port_dev,
136+
+ bool enabled)
137+
+{
138+
+ unsigned long flags;
139+
+
140+
+ uart_port_lock_irqsave(port, &flags);
141+
+ port_dev->tx_enabled = enabled;
142+
+ uart_port_unlock_irqrestore(port, flags);
143+
+}
144+
+
145+
+void serial_base_port_startup(struct uart_port *port)
146+
+{
147+
+ struct serial_port_device *port_dev = port->port_dev;
148+
+
149+
+ serial_base_port_set_tx(port, port_dev, true);
150+
+}
151+
+
152+
+void serial_base_port_shutdown(struct uart_port *port)
153+
+{
154+
+ struct serial_port_device *port_dev = port->port_dev;
155+
+
156+
+ serial_base_port_set_tx(port, port_dev, false);
157+
+}
158+
+
159+
static DEFINE_RUNTIME_DEV_PM_OPS(serial_port_pm,
160+
serial_port_runtime_suspend,
161+
serial_port_runtime_resume, NULL);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <[email protected]>
2+
Date: Wed, 17 Apr 2024 11:03:27 +0200
3+
Subject: [PATCH] serial: stm32: Return IRQ_NONE in the ISR if no handling
4+
happend
5+
MIME-Version: 1.0
6+
Content-Type: text/plain; charset=UTF-8
7+
Content-Transfer-Encoding: 8bit
8+
9+
If there is a stuck irq that the handler doesn't address, returning
10+
IRQ_HANDLED unconditionally makes it impossible for the irq core to
11+
detect the problem and disable the irq. So only return IRQ_HANDLED if
12+
an event was handled.
13+
14+
A stuck irq is still problematic, but with this change at least it only
15+
makes the UART nonfunctional instead of occupying the (usually only) CPU
16+
by 100% and so stall the whole machine.
17+
18+
Fixes: 48a6092fb41f ("serial: stm32-usart: Add STM32 USART Driver")
19+
20+
Signed-off-by: Uwe Kleine-König <[email protected]>
21+
Forwarded: https://lore.kernel.org/r/5f92603d0dfd8a5b8014b2b10a902d91e0bb881f.1713344161.git.u.kleine-koenig@pengutronix.de
22+
---
23+
drivers/tty/serial/stm32-usart.c | 12 ++++++++++--
24+
1 file changed, 10 insertions(+), 2 deletions(-)
25+
26+
diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
27+
index 693e932d6feb..7ea569c681c9 100644
28+
--- a/drivers/tty/serial/stm32-usart.c
29+
+++ b/drivers/tty/serial/stm32-usart.c
30+
@@ -857,6 +857,7 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
31+
const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
32+
u32 sr;
33+
unsigned int size;
34+
+ irqreturn_t ret = IRQ_NONE;
35+
36+
sr = readl_relaxed(port->membase + ofs->isr);
37+
38+
@@ -865,11 +866,14 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
39+
(sr & USART_SR_TC)) {
40+
stm32_usart_tc_interrupt_disable(port);
41+
stm32_usart_rs485_rts_disable(port);
42+
+ ret = IRQ_HANDLED;
43+
}
44+
45+
- if ((sr & USART_SR_RTOF) && ofs->icr != UNDEF_REG)
46+
+ if ((sr & USART_SR_RTOF) && ofs->icr != UNDEF_REG) {
47+
writel_relaxed(USART_ICR_RTOCF,
48+
port->membase + ofs->icr);
49+
+ ret = IRQ_HANDLED;
50+
+ }
51+
52+
if ((sr & USART_SR_WUF) && ofs->icr != UNDEF_REG) {
53+
/* Clear wake up flag and disable wake up interrupt */
54+
@@ -878,6 +882,7 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
55+
stm32_usart_clr_bits(port, ofs->cr3, USART_CR3_WUFIE);
56+
if (irqd_is_wakeup_set(irq_get_irq_data(port->irq)))
57+
pm_wakeup_event(tport->tty->dev, 0);
58+
+ ret = IRQ_HANDLED;
59+
}
60+
61+
/*
62+
@@ -892,6 +897,7 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
63+
uart_unlock_and_check_sysrq(port);
64+
if (size)
65+
tty_flip_buffer_push(tport);
66+
+ ret = IRQ_HANDLED;
67+
}
68+
}
69+
70+
@@ -899,6 +905,7 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
71+
uart_port_lock(port);
72+
stm32_usart_transmit_chars(port);
73+
uart_port_unlock(port);
74+
+ ret = IRQ_HANDLED;
75+
}
76+
77+
/* Receiver timeout irq for DMA RX */
78+
@@ -908,9 +915,10 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
79+
uart_unlock_and_check_sysrq(port);
80+
if (size)
81+
tty_flip_buffer_push(tport);
82+
+ ret = IRQ_HANDLED;
83+
}
84+
85+
- return IRQ_HANDLED;
86+
+ return ret;
87+
}
88+
89+
static void stm32_usart_set_mctrl(struct uart_port *port, unsigned int mctrl)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <[email protected]>
2+
Date: Wed, 17 Apr 2024 11:03:28 +0200
3+
Subject: [PATCH] serial: stm32: Reset .throttled state in .startup()
4+
MIME-Version: 1.0
5+
Content-Type: text/plain; charset=UTF-8
6+
Content-Transfer-Encoding: 8bit
7+
8+
When an UART is opened that still has .throttled set from a previous
9+
open, the RX interrupt is enabled but the irq handler doesn't consider
10+
it. This easily results in a stuck irq with the effect to occupy the CPU
11+
in a tight loop.
12+
13+
So reset the throttle state in .startup() to ensure that RX irqs are
14+
handled.
15+
16+
Fixes: d1ec8a2eabe9 ("serial: stm32: update throttle and unthrottle ops for dma mode")
17+
18+
Signed-off-by: Uwe Kleine-König <[email protected]>
19+
Forwarded: https://lore.kernel.org/r/a784f80d3414f7db723b2ec66efc56e1ad666cbf.1713344161.git.u.kleine-koenig@pengutronix.de
20+
---
21+
drivers/tty/serial/stm32-usart.c | 1 +
22+
1 file changed, 1 insertion(+)
23+
24+
diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
25+
index 7ea569c681c9..d103b07d10ee 100644
26+
--- a/drivers/tty/serial/stm32-usart.c
27+
+++ b/drivers/tty/serial/stm32-usart.c
28+
@@ -1088,6 +1088,7 @@ static int stm32_usart_startup(struct uart_port *port)
29+
val |= USART_CR2_SWAP;
30+
writel_relaxed(val, port->membase + ofs->cr2);
31+
}
32+
+ stm32_port->throttled = false;
33+
34+
/* RX FIFO Flush */
35+
if (ofs->rqr != UNDEF_REG)

0 commit comments

Comments
 (0)