Received-SPF: pass (google.com: domain of linux-kernel+bounces-122395-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Feedback-ID: i58a146ae:Fastmail
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
    Jiri Slaby <jirislaby@kernel.org>
Cc: "Benjamin Herrenschmidt" <benh@kernel.crashing.org>,
    "Michael Ellerman" <mpe@ellerman.id.au>,
    "Nicholas Piggin" <npiggin@gmail.com>,
    "Christophe Leroy" <christophe.leroy@csgroup.eu>,
    "Aneesh Kumar K.V" <aneesh.kumar@kernel.org>,
    "Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
    linux-m68k@lists.linux-m68k.org,
    linux-kernel@vger.kernel.org,
    linux-serial@vger.kernel.org,
    linuxppc-dev@lists.ozlabs.org
Message-Id: <dda2187e128bfaaf092351812e4538e2e41c17f6.1711599093.git.fthain@linux-m68k.org>
From: Finn Thain <fthain@linux-m68k.org>
Subject: [PATCH] serial/pmac_zilog: Remove flawed mitigation for rx irq flood
Date: Thu, 28 Mar 2024 15:11:33 +1100
Precedence: bulk

The mitigation was intended to stop the irq completely. That might have
been better than a hard lock-up but it turns out that you get a crash
anyway if you're using pmac_zilog as a serial console.

That's because the pr_err() call in pmz_receive_chars() results in
pmz_console_write() attempting to lock a spinlock already locked in
pmz_interrupt(). With CONFIG_DEBUG_SPINLOCK=y, this produces a fatal
BUG splat like the one below. (The spinlock at 0x62e140 is the one in
struct uart_port.)

Even when it's not fatal, the serial port rx function ceases to work.
Also, the iteration limit doesn't play nicely with QEMU. Please see
bug report linked below.

A web search for reports of the error message "pmz: rx irq flood" didn't
produce anything. So I don't think this code is needed any more. Remove it.

[   14.560000] ttyPZ0: pmz: rx irq flood !
[   14.560000] BUG: spinlock recursion on CPU#0, swapper/0
[   14.560000]  lock: 0x62e140, .magic: dead4ead, .owner: swapper/0, .owner_cpu: 0
[   14.560000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.8.0-mac-dbg-preempt-00004-g4143b7b9144a #1
[   14.560000] Stack from 0059bcc4:
[   14.560000]         0059bcc4 0056316f 0056316f 00002700 004b6444 0059bce4 004ad8c6 0056316f
[   14.560000]         0059bd10 004a6546 00556759 0062e140 dead4ead 0059f892 00000000 00000000
[   14.560000]         0062e140 0059bde8 005c03d0 0059bd24 0004daf6 0062e140 005567bf 0062e140
[   14.560000]         0059bd34 004b64c2 0062e140 00000001 0059bd50 002e15ea 0062e140 00000001
[   14.560000]         0059bde7 0059bde8 005c03d0 0059bdac 0005124e 005c03d0 005cdc00 0000002b
[   14.560000]         005a3caa 005a3caa 00000000 0059bde8 0004ff00 0059be8b 00038200 000529ba
[   14.560000] Call Trace: [<00002700>] ret_from_kernel_thread+0xc/0x14
[   14.560000]  [<004b6444>] _raw_spin_lock+0x0/0x28
[   14.560000]  [<004ad8c6>] dump_stack+0x10/0x16
[   14.560000]  [<004a6546>] spin_dump+0x6e/0x7c
[   14.560000]  [<0004daf6>] do_raw_spin_lock+0x9c/0xa6
[   14.560000]  [<004b64c2>] _raw_spin_lock_irqsave+0x2a/0x34
[   14.560000]  [<002e15ea>] pmz_console_write+0x32/0x9a
[   14.560000]  [<0005124e>] console_flush_all+0x112/0x3a2
[   14.560000]  [<0004ff00>] console_trylock+0x0/0x7a
[   14.560000]  [<00038200>] parameq+0x48/0x6e
[   14.560000]  [<000529ba>] __printk_safe_enter+0x0/0x36
[   14.560000]  [<0005113c>] console_flush_all+0x0/0x3a2
[   14.560000]  [<000542c4>] prb_read_valid+0x0/0x1a
[   14.560000]  [<004b65a4>] _raw_spin_unlock+0x0/0x38
[   14.560000]  [<0005151e>] console_unlock+0x40/0xb8
[   14.560000]  [<00038200>] parameq+0x48/0x6e
[   14.560000]  [<002c778c>] __tty_insert_flip_string_flags+0x0/0x14e
[   14.560000]  [<00051798>] vprintk_emit+0x156/0x238
[   14.560000]  [<00051894>] vprintk_default+0x1a/0x1e
[   14.560000]  [<000529a8>] vprintk+0x74/0x86
[   14.560000]  [<004a6596>] _printk+0x12/0x16
[   14.560000]  [<002e23be>] pmz_receive_chars+0x1cc/0x394
[   14.560000]  [<004b6444>] _raw_spin_lock+0x0/0x28
[   14.560000]  [<00038226>] parse_args+0x0/0x3a6
[   14.560000]  [<004b6466>] _raw_spin_lock+0x22/0x28
[   14.560000]  [<002e26b4>] pmz_interrupt+0x12e/0x1e0
[   14.560000]  [<00048680>] arch_cpu_idle_enter+0x0/0x8
[   14.560000]  [<00054ebc>] __handle_irq_event_percpu+0x24/0x106
[   14.560000]  [<004ae576>] default_idle_call+0x0/0x46
[   14.560000]  [<00055020>] handle_irq_event+0x30/0x90
[   14.560000]  [<00058320>] handle_simple_irq+0x5e/0xc0
[   14.560000]  [<00048688>] arch_cpu_idle_exit+0x0/0x8
[   14.560000]  [<00054800>] generic_handle_irq+0x3c/0x4a
[   14.560000]  [<00002978>] do_IRQ+0x24/0x3a
[   14.560000]  [<004ae508>] cpu_idle_poll.isra.0+0x0/0x6e
[   14.560000]  [<00002874>] auto_irqhandler_fixup+0x4/0xc
[   14.560000]  [<004ae508>] cpu_idle_poll.isra.0+0x0/0x6e
[   14.560000]  [<004ae576>] default_idle_call+0x0/0x46
[   14.560000]  [<004ae598>] default_idle_call+0x22/0x46
[   14.560000]  [<00048710>] do_idle+0x6a/0xf0
[   14.560000]  [<000486a6>] do_idle+0x0/0xf0
[   14.560000]  [<000367d2>] find_task_by_pid_ns+0x0/0x2a
[   14.560000]  [<0005d064>] __rcu_read_lock+0x0/0x12
[   14.560000]  [<00048a5a>] cpu_startup_entry+0x18/0x1c
[   14.560000]  [<00063a06>] __rcu_read_unlock+0x0/0x26
[   14.560000]  [<004ae65a>] kernel_init+0x0/0xfa
[   14.560000]  [<0049c5a8>] strcpy+0x0/0x1e
[   14.560000]  [<004a6584>] _printk+0x0/0x16
[   14.560000]  [<0049c72a>] strlen+0x0/0x22
[   14.560000]  [<006452d4>] memblock_alloc_try_nid+0x0/0x82
[   14.560000]  [<0063939a>] arch_post_acpi_subsys_init+0x0/0x8
[   14.560000]  [<0063991e>] console_on_rootfs+0x0/0x60
[   14.560000]  [<00638410>] _sinittext+0x410/0xadc
[   14.560000]

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@kernel.org>
Cc: "Naveen N. Rao" <naveen.n.rao@linux.ibm.com>
Cc: linux-m68k@lists.linux-m68k.org
Link: https://github.com/vivier/qemu-m68k/issues/44
Link: https://lore.kernel.org/all/1078874617.9746.36.camel@gaston/
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
 drivers/tty/serial/pmac_zilog.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
index c8bf08c19c64..77691fbbf779 100644
--- a/drivers/tty/serial/pmac_zilog.c
+++ b/drivers/tty/serial/pmac_zilog.c
@@ -210,7 +210,6 @@ static bool pmz_receive_chars(struct uart_pmac_port *uap)
 {
 	struct tty_port *port;
 	unsigned char ch, r1, drop, flag;
-	int loops = 0;
 
 	/* Sanity check, make sure the old bug is no longer happening */
 	if (uap->port.state == NULL) {
@@ -291,24 +290,11 @@ static bool pmz_receive_chars(struct uart_pmac_port *uap)
 		if (r1 & Rx_OVR)
 			tty_insert_flip_char(port, 0, TTY_OVERRUN);
 	next_char:
-		/* We can get stuck in an infinite loop getting char 0 when the
-		 * line is in a wrong HW state, we break that here.
-		 * When that happens, I disable the receive side of the driver.
-		 * Note that what I've been experiencing is a real irq loop where
-		 * I'm getting flooded regardless of the actual port speed.
-		 * Something strange is going on with the HW
-		 */
-		if ((++loops) > 1000)
-			goto flood;
 		ch = read_zsreg(uap, R0);
 		if (!(ch & Rx_CH_AV))
 			break;
 	}
 
-	return true;
- flood:
-	pmz_interrupt_control(uap, 0);
-	pmz_error("pmz: rx irq flood !\n");
 	return true;
 }
 
-- 
2.39.3