From patchwork Fri Jul 19 13:35:24 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Martin X-Patchwork-Id: 11050117 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id DA62B14F6 for ; Fri, 19 Jul 2019 13:36:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C8BB4286D1 for ; Fri, 19 Jul 2019 13:36:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BD4AF28950; Fri, 19 Jul 2019 13:36:27 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 504E22878E for ; Fri, 19 Jul 2019 13:36:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:References: In-Reply-To:Message-Id:Date:Subject:To:From:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=o4isfWZjM3GAfGFhzhqR8xkz/BzI1R8YJeiZLzL1GiQ=; b=exUdwrflY/E3Eg3eE73z+A58VP hR7uhkn0CkChrj/rAqG3QCin+P97K2DlXay1Ho1WKgFryDsw0/Vbf135yAZ7CINfhSeY640lTShbh FWK53gXHF/sD0xu79bZ4sqMXECwDVMlp8e4ejEQUy5nJK2DBPeO1IxW+oVppbJwpielceOdRmcfRe dyiuzj0huO+3qtBsrH8iBn2YgnS4wiu0kNnB5XdkrRdm0f1df1xNA4hRDO5Oy/KpYKITXmDiBWjq9 l0Z07S4u9gM/tt+u4lzFZBeXKzP9iszK6DWTPEUjsMhtGhF+yxsw6tKGGwfxJsMr1LiWicQP2Wmwm /f6CX6Yg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1hoT3i-0006AM-Rm; Fri, 19 Jul 2019 13:36:26 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1hoT3f-00069E-5y; Fri, 19 Jul 2019 13:36:24 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2EF2D337; Fri, 19 Jul 2019 06:36:22 -0700 (PDT) Received: from e103592.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 110273F71A; Fri, 19 Jul 2019 06:36:20 -0700 (PDT) From: Dave Martin To: linux-serial@vger.kernel.org Subject: [RFC PATCH 1/2] serial: pl011: Fix dropping of TX chars due to irq race Date: Fri, 19 Jul 2019 14:35:24 +0100 Message-Id: <1563543325-12463-2-git-send-email-Dave.Martin@arm.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1563543325-12463-1-git-send-email-Dave.Martin@arm.com> References: <1563543325-12463-1-git-send-email-Dave.Martin@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190719_063623_268260_361AA186 X-CRM114-Status: UNSURE ( 9.39 ) X-CRM114-Notice: Please train this message. X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Russell King , Greg Kroah-Hartman , Phil Elwell , linux-rpi-kernel@lists.infradead.org, Jiri Slaby , Rogier Wolff , linux-arm-kernel@lists.infradead.org MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP When serial_core pushes some new TX chars via a call to pl011_start_tx(), it can race with irqs triggered by ongoing transmission. Normally the port lock protects against this kind of thing, but temporary releasing of the lock during calls from pl011_int() to pl011_{,dma_}rx_chars() allows pl011_start_tx() to race. For performance reasons, pl011_tx_chars(, true) always assumes that the TX FIFO interrupt trigger condition holds, i.e., the FIFO is empty to the trigger threshold. This means that we can write chars to fill the FIFO back up without the expense of polling the FIFO fill status. However, this assumes that no data is written to the FIFO in the meantime by other code: this is where the race with pl011_start_tx_pio() breaks things. Reorder pl011_int() so that no code releases the port lock in between reading the interrupt status bits and calling pl011_tx_chars(). This ensures that TXIS in the fetched status accurately reflects the state of the TX FIFO, and ensures that there is no race to fill the FIFO. Fixes: 1e84d22322ce ("serial/amba-pl011: Refactor and simplify TX FIFO handling") Reported-by: Phil Elwell Signed-off-by: Dave Martin --- drivers/tty/serial/amba-pl011.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index 89ade21..e24bbc0 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -1492,6 +1492,13 @@ static irqreturn_t pl011_int(int irq, void *dev_id) UART011_RXIS), uap, REG_ICR); + /* + * Don't unlock uap->port.lock before here: + * Stale TXIS status can lead to a FIFO overfill. + */ + if (status & UART011_TXIS) + pl011_tx_chars(uap, true); + if (status & (UART011_RTIS|UART011_RXIS)) { if (pl011_dma_rx_running(uap)) pl011_dma_rx_irq(uap); From patchwork Fri Jul 19 13:35:25 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Martin X-Patchwork-Id: 11050123 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3295213B1 for ; Fri, 19 Jul 2019 13:36:55 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 20F2928635 for ; Fri, 19 Jul 2019 13:36:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 155EC28702; Fri, 19 Jul 2019 13:36:55 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id AFDF9286D1 for ; Fri, 19 Jul 2019 13:36:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:References: In-Reply-To:Message-Id:Date:Subject:To:From:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=5eylCfc+DTJ5PmhnsoI7h8w4hdBYDs7KmHiQqsHaN6w=; b=IIE8ZPGurpQ6RPWjpZjDpZ1NsA n9rxS57hQ7TMrTf6nT5UnXybu5+rU+9a5SNr94CEXbmLXerET99fgLvNS+Fde4zrnuMXlZO+FeQUc o+aEh1ilb8wA/jFVRYtqH/+u0IdYYlcyTLuDbsPQrNzvRzWmssxQqBQUUd8g3xJhC2I1SBMSCpVjj n3OYm33EkQymVaVSy4fzZb3BUjFeqqwnyQ6gXwRfpU1q2KdiMj2btIhs1E4OZVudHNJ1U830Pa8Wf G01ck1typ2t2zjq5qciK5qDHe/eZLmljhpDlhueeFekhkO8zA6xboBaLPHsm7eLaYqA01RGskB5tQ hDTJNkQg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1hoT49-0006QD-V2; Fri, 19 Jul 2019 13:36:54 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1hoT3f-00069a-W8; Fri, 19 Jul 2019 13:36:25 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8BC3D1509; Fri, 19 Jul 2019 06:36:23 -0700 (PDT) Received: from e103592.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 642693F71A; Fri, 19 Jul 2019 06:36:22 -0700 (PDT) From: Dave Martin To: linux-serial@vger.kernel.org Subject: [RFC PATCH 2/2] serial: pl011: Don't bother pushing more TX data while TX irq is active Date: Fri, 19 Jul 2019 14:35:25 +0100 Message-Id: <1563543325-12463-3-git-send-email-Dave.Martin@arm.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1563543325-12463-1-git-send-email-Dave.Martin@arm.com> References: <1563543325-12463-1-git-send-email-Dave.Martin@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190719_063624_071274_406A50F3 X-CRM114-Status: GOOD ( 12.20 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Russell King , Greg Kroah-Hartman , Phil Elwell , linux-rpi-kernel@lists.infradead.org, Jiri Slaby , Rogier Wolff , linux-arm-kernel@lists.infradead.org MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP When the TX irq is active, writing chars to the TX FIFO from anywhere except pl011_int() is pointless: the UART is already busy, and new chars will be picked up by pl011_int() as soon as there is FIFO space. To reduce the scope for surprises, bail out of pl011_start_tx_pio() without attempting to write to the FIFO or start TX DMA if the TX FIFO interrupt is already in use. This should also avoid pointless overhead in some situations. Signed-off-by: Dave Martin --- Please test both with and without this patch. I believe with the previous patch in place, this patch is not strictly necessary. However, if the UART is actively transmitting in the background already, it does make sense not to waste time trying polling the FIFO fill status or setting up DMA etc. --- drivers/tty/serial/amba-pl011.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index e24bbc0..f28935a 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -1318,6 +1318,10 @@ static void pl011_start_tx(struct uart_port *port) struct uart_amba_port *uap = container_of(port, struct uart_amba_port, port); + /* It's pointless to kick the UART if it's already transmitting... */ + if (uap->im & UART011_TXIM) + return; + if (!pl011_dma_tx_start(uap)) pl011_start_tx_pio(uap); }