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: 169251 Delivered-To: patch@linaro.org Received: by 2002:a92:4782:0:0:0:0:0 with SMTP id e2csp3873979ilk; Fri, 19 Jul 2019 06:36:24 -0700 (PDT) X-Google-Smtp-Source: APXvYqyHeHj+NYjW4VYjsQKB/E8YhdUPlD7wh/TztGroB6pyD1BIvHZsJ7vS1JXi+6G4I7/Trted X-Received: by 2002:a63:c64b:: with SMTP id x11mr54074291pgg.319.1563543383837; Fri, 19 Jul 2019 06:36:23 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1563543383; cv=none; d=google.com; s=arc-20160816; b=vHAVoPqRjJqeA3GHV1VFzB0efvC5X2+OBEFop0Zqt9SPcTJT35M1qH+p7KjeiNlWAH E6W9JrLZxSzar1AvBfUehg4ADwftHaV8PDN5R+UmaedoWvMOkuIeWlWqAupKpbad9buW mttitZwgmG39lWjnKXEBjhilJX2bX/EveoBfEKgbNPtLH1cocwcUXA7BskK9Vy+7ZCSA 9OVBtCW7LyeOYel07h4S4zlzIdpxyCKEprzyFdfGpInGolGc47EvBbTRh9o/rhb3ygD3 xbxKrd17lEt/dUqRENBTr8ltlLhRMO1aAxZzLVYvk+fWqDVfaV32Vzk2GGA73LAYXKWo d1TA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:references:in-reply-to:message-id:date :subject:cc:to:from; bh=ENwsPftZ+RdZUxDsie/qUc44+Lkcb4+gg/zkhOi86/o=; b=YNaihmYdI0rU+rEBk5H8C+wmNef6s677lrDhl3eeyqB116Jrj7FajvpaSiwIX8cRSi Y03uitDo0O8F30l05mJCtjzQrECfN6CkFVpEd1wL9z2KvNPcMGNgP77bsHQsd16Bfp/T vOcF3K+1hHPP+TehCXRP3F/jwwEui3bRUl7nQylRTNJYeS6/5cTikVJ/6KmAY/wCJoZA gwmUqffhDC8pMUP7eTQjd2Kt/x4lxytXmTMY4HFky74zG/hox3iHL3g6/X+tAJpvNWil aT9bY3k12V/JGvZiOxzq0i0d9YtQSdMY3QhReOTI/W7Q2PkdSWccdLIm0ybKhH4a5Nzt uRow== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-serial-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-serial-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id n12si1905954pgi.550.2019.07.19.06.36.23; Fri, 19 Jul 2019 06:36:23 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-serial-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-serial-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-serial-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728003AbfGSNgX (ORCPT + 1 other); Fri, 19 Jul 2019 09:36:23 -0400 Received: from foss.arm.com ([217.140.110.172]:43508 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727888AbfGSNgX (ORCPT ); Fri, 19 Jul 2019 09:36:23 -0400 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 Cc: Russell King , Phil Elwell , Rogier Wolff , linux-arm-kernel@lists.infradead.org, Greg Kroah-Hartman , Jiri Slaby , linux-rpi-kernel@lists.infradead.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> Sender: linux-serial-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-serial@vger.kernel.org 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(+) -- 2.1.4 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: 169252 Delivered-To: patch@linaro.org Received: by 2002:a92:4782:0:0:0:0:0 with SMTP id e2csp3874010ilk; Fri, 19 Jul 2019 06:36:25 -0700 (PDT) X-Google-Smtp-Source: APXvYqxbVCjEguKBFuEJrSQuvcjr76opb+EWdCNCWfjDJDylp3xKjjhJviuF9183pbzlUdnTO+qO X-Received: by 2002:a17:90a:80c4:: with SMTP id k4mr58589604pjw.74.1563543385061; Fri, 19 Jul 2019 06:36:25 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1563543385; cv=none; d=google.com; s=arc-20160816; b=AcE/0CmHVBrPoSlX74uonqoJgd1GQ1tZ4oFDXuEQdMM33M5W6KUUcsuwAdwvZOUF5t qIIO0E1NB10l8gbkbZqHHXbTrWbyoI56enWoaNuxLKkVou8RY9sWDG6JtO/2Am28itLT 7sFwNG0y1n8Cz6/2dFlJQfpNP1k6sKYsKRHQd9Ae1jl6KkqedlJDVCW2a7sEzYm3Jz0S lcbaJKl7cmcLnW80cHoOH6B/gzH6eySa+WDxPLSX+8AL6eo2dbF3qxk4C4Xa9pAYznL5 mW8rzujbQnSw2IXOT1ITp0P5lvHVwpuBYdC4GUXxBZwfxyTXrbcGUdxGB6pD7/i/5RH6 aBsQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:references:in-reply-to:message-id:date :subject:cc:to:from; bh=TJk1L971VybrFq6TbR869lC19j+d8ZGEPtKJvHMEZaA=; b=d75+qKWt+sXQ25BCBVce59wsBqUVq9ug44acXY5amdR95yCWNi9gCWFhKeivaAHLxk eC5YAoQTjHLvoxJpI89hQj4krD1Ik0HJ0FErTfNj5J9spAHQtbx09tQHp1aw2rTL9BTp dAUcTSSDvPA1h+peyaOXKG97n16bzF3PtbC9zIHUkaS7eBEbRVuBPP4sIsGLH2oRhnsu yuNJv7uZDMyU0WaBGm2U5rZCDEAJ/WvYb1hoRxhB1/zcve8Sh9Aek9b0CbCfsUEbUASs oN12KSG6JGrkeI2QwgTpQbRaXsbELcB39d80ltjxJmqNuuRhQ5ecSXXi8FMizNV4/CtK NYZw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-serial-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-serial-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id r14si2278696pgi.513.2019.07.19.06.36.24; Fri, 19 Jul 2019 06:36:25 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-serial-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-serial-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-serial-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728016AbfGSNgY (ORCPT + 1 other); Fri, 19 Jul 2019 09:36:24 -0400 Received: from foss.arm.com ([217.140.110.172]:43522 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727888AbfGSNgY (ORCPT ); Fri, 19 Jul 2019 09:36:24 -0400 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 Cc: Russell King , Phil Elwell , Rogier Wolff , linux-arm-kernel@lists.infradead.org, Greg Kroah-Hartman , Jiri Slaby , linux-rpi-kernel@lists.infradead.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> Sender: linux-serial-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-serial@vger.kernel.org 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(+) -- 2.1.4 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); }