mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Samarth Narang <samarth.colleges@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>
Cc: Samarth Narang <samarth.colleges@gmail.com>,
	Russell King <linux@armlinux.org.uk>, Frank Li <Frank.Li@nxp.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	linux-serial@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 1/2] serial: amba-pl011: keep the flushed TX buffer mapped until DMA stops
Date: Sat, 31 Oct 2026 12:37:48 +0000	[thread overview]
Message-ID: <20261031123749.3783352-2-samarth.colleges@gmail.com> (raw)
In-Reply-To: <20261031123749.3783352-1-samarth.colleges@gmail.com>

pl011_dma_flush_buffer() is called with the port lock held and currently
unmaps the TX buffer immediately after requesting termination with
dmaengine_terminate_async(). Since asynchronous termination does not
guarantee that the transfer or its completion callback has finished, the
DMA controller may continue reading from the buffer after its mapping
has been released. Commit f6a196477184 ("serial: pl011: Fix DMA
->flush_buffer()") introduced asynchronous termination under the port
lock, but did not add the required synchronization before unmapping.

On systems using an IOMMU or SWIOTLB, outstanding DMA reads may access
an IOVA or bounce buffer that has already been reused, potentially
transmitting data from another mapping. On systems without either, the
outstanding transfer may instead race with the next refill of dmatx.buf.
This can be triggered by flushing the TTY while TX DMA is active.

Since flush_buffer() cannot sleep, defer the unmap to a work item that
calls dmaengine_synchronize() before releasing the mapping. Keep
dmatx.queued set to prevent buffer reuse, disable TX DMA requests, and
let the work item restart transmission if data remains pending. The
completion callback leaves flushed transfers for deferred cleanup, while
shutdown waits for the work to finish before tearing down TX DMA.

Fixes: f6a196477184 ("serial: pl011: Fix DMA ->flush_buffer()")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Samarth Narang <samarth.colleges@gmail.com>
---
 drivers/tty/serial/amba-pl011.c | 54 +++++++++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index e6f652905ca6..863a050cf654 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -41,6 +41,7 @@
 #include <linux/sizes.h>
 #include <linux/io.h>
 #include <linux/acpi.h>
+#include <linux/workqueue.h>
 
 #define UART_NR			14
 
@@ -302,6 +303,8 @@ struct pl011_dmatx_data {
 	size_t			len;
 	char			*buf;
 	bool			queued;
+	bool			flushing;
+	struct work_struct	flush_work;
 };
 
 enum pl011_rs485_tx_state {
@@ -609,6 +612,12 @@ static void pl011_dma_tx_callback(void *data)
 	u16 dmacr;
 
 	uart_port_lock_irqsave(&uap->port, &flags);
+	if (uap->dmatx.flushing) {
+		/* pl011_dma_tx_flush_work() retires the flushed buffer */
+		uart_port_unlock_irqrestore(&uap->port, flags);
+		return;
+	}
+
 	if (uap->dmatx.queued)
 		dma_unmap_single(dmaengine_get_dma_device(dmatx->chan), dmatx->dma,
 				 dmatx->len, DMA_TO_DEVICE);
@@ -867,13 +876,47 @@ __acquires(&uap->port.lock)
 
 	dmaengine_terminate_async(uap->dmatx.chan);
 
-	if (uap->dmatx.queued) {
-		dma_unmap_single(dmaengine_get_dma_device(uap->dmatx.chan), uap->dmatx.dma,
-				 uap->dmatx.len, DMA_TO_DEVICE);
+	/*
+	 * The terminated transfer may still be reading dmatx.buf: keep it
+	 * mapped, and queued set so that nothing refills it, until
+	 * pl011_dma_tx_flush_work() has synchronized the channel.
+	 */
+	if (uap->dmatx.queued && !uap->dmatx.flushing) {
+		uap->dmatx.flushing = true;
+		uap->dmacr &= ~UART011_TXDMAE;
+		pl011_write(uap->dmacr, uap, REG_DMACR);
+		schedule_work(&uap->dmatx.flush_work);
+	}
+}
+
+/*
+ * flush_buffer runs under the port lock and can only request termination.
+ * Unmap the flushed TX buffer here, once the terminated transfer and its
+ * completion callback are known to have finished, then restart TX.
+ */
+static void pl011_dma_tx_flush_work(struct work_struct *work)
+{
+	struct uart_amba_port *uap =
+	    container_of(work, struct uart_amba_port, dmatx.flush_work);
+	struct tty_port *tport = &uap->port.state->port;
+	unsigned long flags;
+
+	dmaengine_synchronize(uap->dmatx.chan);
+
+	uart_port_lock_irqsave(&uap->port, &flags);
+	if (uap->dmatx.flushing) {
+		dma_unmap_single(dmaengine_get_dma_device(uap->dmatx.chan),
+				 uap->dmatx.dma, uap->dmatx.len, DMA_TO_DEVICE);
+		uap->dmatx.flushing = false;
 		uap->dmatx.queued = false;
 		uap->dmacr &= ~UART011_TXDMAE;
 		pl011_write(uap->dmacr, uap, REG_DMACR);
+		if (!uart_tx_stopped(&uap->port) &&
+		    !kfifo_is_empty(&tport->xmit_fifo) &&
+		    pl011_dma_tx_refill(uap) <= 0)
+			pl011_start_tx_pio(uap);
 	}
+	uart_port_unlock_irqrestore(&uap->port, flags);
 }
 
 static void pl011_dma_rx_callback(void *data);
@@ -1177,6 +1220,8 @@ static void pl011_dma_startup(struct uart_amba_port *uap)
 
 	/* The DMA buffer is now the FIFO the TTY subsystem can use */
 	uap->port.fifosize = PL011_DMA_BUFFER_SIZE;
+	uap->dmatx.flushing = false;
+	INIT_WORK(&uap->dmatx.flush_work, pl011_dma_tx_flush_work);
 	uap->using_tx_dma = true;
 
 	if (!uap->dmarx.chan)
@@ -1236,6 +1281,9 @@ static void pl011_dma_shutdown(struct uart_amba_port *uap)
 	if (!(uap->using_tx_dma || uap->using_rx_dma))
 		return;
 
+	if (uap->using_tx_dma)
+		flush_work(&uap->dmatx.flush_work);
+
 	/* Disable RX and TX DMA */
 	while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy)
 		cpu_relax();
-- 
2.34.1


  reply	other threads:[~2026-10-31 12:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-31  0:54 [BUG] serial: amba-pl011, imx, fsl_lpuart: ->flush_buffer() unmaps the TX DMA buffer before the transfer has stopped Samarth Narang
2026-10-31  8:55 ` Greg Kroah-Hartman
2026-10-31 12:37 ` [PATCH 0/2] serial: keep the flushed TX buffer mapped until DMA " Samarth Narang
2026-10-31 12:37   ` Samarth Narang [this message]
2026-10-31 12:37   ` [PATCH 2/2] tty: serial: fsl_lpuart: don't unmap TX buffer before " Samarth Narang
2026-10-31 13:00   ` [PATCH 0/2] serial: keep the flushed TX buffer mapped until " Greg Kroah-Hartman
2026-10-31 13:44     ` Samarth Narang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261031123749.3783352-2-samarth.colleges@gmail.com \
    --to=samarth.colleges@gmail.com \
    --cc=Frank.Li@nxp.com \
    --cc=festevam@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=imx@lists.linux.dev \
    --cc=jirislaby@kernel.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=s.hauer@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®