From patchwork Sun May 8 10:35:47 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Ogness X-Patchwork-Id: 12842326 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 43669C433EF for ; Sun, 8 May 2022 10:37:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-Id:Date:Subject:Cc :To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=rujnDOoP2lx06BiTpYUQ9DJ6U5g3ETuFWgb2ogWk1UY=; b=oRkJhXHX26hMYf Kr0oCarP3cGpMKZY5YGY225dRF9sywYylih9UIjR/7JoY8ps3BQK7kEGCT7e18oUMFH9XAZTj33kB rwb0xaQ40uzd9FG6VUy0jrjUyV2R+Sk4p9642Qx1xRpYKXH5t0H4w9eSlBibAn/Pnop/FJw6nmsUa v+P/FEZ1K4NeglX2YnzlfrTu4AXmlSS7lz+jqNlu4/r7wbbbq0F3MRxzpF/HZe1PJO+qSyoecZVvS 6GbSXP3grFbOzTatwdgHL/sm0cfbiZfSqJKgH7XUpDDFjgF9An90AgRNZwUAi8vf1OBfz/NRgFBvU 0yMD1AcWHFN7NtrJH1jg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nneGb-009qhC-FT; Sun, 08 May 2022 10:35:57 +0000 Received: from galois.linutronix.de ([193.142.43.55]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nneGX-009qew-2i; Sun, 08 May 2022 10:35:54 +0000 From: John Ogness DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1652006148; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=Wg5rgmNfLBMFs6+g076Tc69lJyydQy68rcQ9tdaUcOE=; b=d8FMbrH+ZnIzNn38VNXgyx0rlmVihz2sgmBykxM1ijuFHr/encYlNAO/FYu+00jN5IaBPf nuT70471orDPF3hqmVgafDF+6S44F0DBmgHNcDDXybOzpk2dytgoLzeWg35VQJW5rLBx+2 CO2hxm90hqJoqW+/c+6edL583AnVr+IvIHWQl7KMaT+LqnoP5+WhaqKm/i2FZMcKyqtQON 2lu8+YOHSNxdzeYuGUZ+9xA9ZPKMGN9VYjrNAhLH9k9izTmalcjST1YecYH4ZRm3Vc9+47 wCZrxf+vm13kfAZMwgZoV/uSs3Lp8leLOcTpXlMrXKVYhTN2d9FgXEY0t6sXpA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1652006148; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=Wg5rgmNfLBMFs6+g076Tc69lJyydQy68rcQ9tdaUcOE=; b=jbctOsS1Qy4kMsGrmyICmp7zgkryPYX6MCehq3ekzyo01N9EyO256vJmZBPTyf8dl2IGuw 1Usb4/w4051eRfBg== To: "Greg Kroah-Hartman" , Neil Armstrong , Kevin Hilman Cc: Petr Mladek , Sergey Senozhatsky , Steven Rostedt , linux-kernel@vger.kernel.org, Jiri Slaby , Jerome Brunet , Martin Blumenstingl , linux-serial@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-amlogic@lists.infradead.org, Marek Szyprowski Subject: [PATCH v1] serial: meson: acquire port->lock in startup() Date: Sun, 8 May 2022 12:41:47 +0206 Message-Id: <20220508103547.626355-1-john.ogness@linutronix.de> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220508_033553_355673_0DAFC2BE X-CRM114-Status: GOOD ( 13.03 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org The uart_ops startup() callback is called without interrupts disabled and without port->lock locked, relatively late during the boot process (from the call path of console_on_rootfs()). If the device is a console, it was already previously registered and could be actively printing messages. Since the startup() callback is reading/writing registers used by the console write() callback (AML_UART_CONTROL), its access must be synchronized using the port->lock. Currently it is not. The startup() callback is the only function that explicitly enables interrupts. Without the synchronization, it is possible that interrupts become accidentally permanently disabled. CPU0 CPU1 meson_serial_console_write meson_uart_startup -------------------------- ------------------ spin_lock(port->lock) val = readl(AML_UART_CONTROL) uart_console_write() writel(INT_EN, AML_UART_CONTROL) writel(val, AML_UART_CONTROL) spin_unlock(port->lock) Add port->lock synchronization to meson_uart_startup() to avoid racing with meson_serial_console_write(). Also add detailed comments to meson_uart_reset() explaining why it is *not* using port->lock synchronization. Link: https://lore.kernel.org/lkml/2a82eae7-a256-f70c-fd82-4e510750906e@samsung.com Reported-by: Marek Szyprowski Signed-off-by: John Ogness Tested-by: Marek Szyprowski Reviewed-by: Jiri Slaby Acked-by: Neil Armstrong Reviewed-by: Petr Mladek --- drivers/tty/serial/meson_uart.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) base-commit: 672c0c5173427e6b3e2a9bbb7be51ceeec78093a diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c index 2bf1c57e0981..39021dac09cc 100644 --- a/drivers/tty/serial/meson_uart.c +++ b/drivers/tty/serial/meson_uart.c @@ -253,6 +253,14 @@ static const char *meson_uart_type(struct uart_port *port) return (port->type == PORT_MESON) ? "meson_uart" : NULL; } +/* + * This function is called only from probe() using a temporary io mapping + * in order to perform a reset before setting up the device. Since the + * temporarily mapped region was successfully requested, there can be no + * console on this port at this time. Hence it is not necessary for this + * function to acquire the port->lock. (Since there is no console on this + * port at this time, the port->lock is not initialized yet.) + */ static void meson_uart_reset(struct uart_port *port) { u32 val; @@ -267,9 +275,12 @@ static void meson_uart_reset(struct uart_port *port) static int meson_uart_startup(struct uart_port *port) { + unsigned long flags; u32 val; int ret = 0; + spin_lock_irqsave(&port->lock, flags); + val = readl(port->membase + AML_UART_CONTROL); val |= AML_UART_CLEAR_ERR; writel(val, port->membase + AML_UART_CONTROL); @@ -285,6 +296,8 @@ static int meson_uart_startup(struct uart_port *port) val = (AML_UART_RECV_IRQ(1) | AML_UART_XMIT_IRQ(port->fifosize / 2)); writel(val, port->membase + AML_UART_MISC); + spin_unlock_irqrestore(&port->lock, flags); + ret = request_irq(port->irq, meson_uart_interrupt, 0, port->name, port);