[PATCH 0/2] spi: tegra210-quad: Fix timeout path status handling and DMA leak

[PATCH 0/2] spi: tegra210-quad: Fix timeout path status handling and DMA leak

[Vishwaroop A]

Two independent fixes in the Tegra QSPI transfer timeout path.

Patch 1 fixes a regression from commit 1ff82c13ea94 ("spi: tegra210-quad:
Cache TRANS_STATUS in ISR for timeout handler"), currently in linux-next
only. When tegra_qspi_handle_timeout() observes a real ISR completion
after draining the workqueue, it still publishes the pre-drain lost-IRQ
FIFO snapshot. That snapshot can hold zeroed error bits if the ISR
W1C-ed the hardware between the two reads, which makes the manual
completion handler downstream accept a transfer that actually reported a
FIFO underflow or overflow. No stable backport is needed because the
offending commit has not been released.

Patch 2 fixes a DMA mapping leak present since the driver was added.
tegra_qspi_dma_map_xfer() maps the client TX/RX buffers, but the only
unmap lives in handle_dma_based_xfer(), which runs from the interrupt
bottom half. On a genuine timeout the bottom half never runs and both
transfer paths abort with tegra_qspi_dma_stop() plus a controller reset
without unmapping, so every real timeout leaks the streaming mappings
for that chunk. On systems with an IOMMU in front of QSPI this
gradually exhausts the IOVA space. This one is tagged for stable.

Both issues surfaced while reviewing a downstream backport of the
hard-IRQ series now in linux-next; patch 1 carries the corresponding
Reported-by and Link tags.

Vishwaroop A (2):
  spi: tegra210-quad: don't overwrite refreshed status with lost-IRQ
    snapshot
  spi: tegra210-quad: Unmap DMA buffers on transfer timeout

 drivers/spi/spi-tegra210-quad.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

-- 
2.17.1

[PATCH 1/2] spi: tegra210-quad: don't overwrite refreshed status with lost-IRQ snapshot

[Vishwaroop A]

tegra_qspi_handle_timeout() captures a "lost IRQ" FIFO status snapshot
when the ISR cache is empty on entry but QSPI_TRANS_STATUS shows the
transfer completed. After masking the controller IRQ and draining the
workqueue with cancel_work_sync(), it re-loads the ISR cache with
smp_load_acquire(): if the drain observed a real ISR completion, the
refreshed cache holds ISR-published status_reg, tx_status and rx_status
that reflect the true hardware error state at IRQ time.

However, when both cache and snapshot are set, the code keeps the
refreshed value in the local status variable but then unconditionally
publishes the pre-drain lost_fifo_status / lost_tx_status /
lost_rx_status snapshot into tqspi->status_reg / tx_status / rx_status
via the lost_irq_snapshot branch. That snapshot is racy: if the ISR
fires on another CPU between our live QSPI_TRANS_STATUS read and our
live QSPI_FIFO_STATUS snapshot, the ISR W1Cs the hardware error bits
before we sample them, so the snapshot may contain zeroed error
fields. Overwriting the ISR-published values with these zeros makes
the manual completion handler downstream silently accept a transfer
that actually reported a FIFO error.

Clear lost_irq_snapshot when the refreshed cache is non-zero, so the
ISR-published status wins over the pre-drain snapshot in that case.
The lost-IRQ path (refreshed == 0 && live QSPI_RDY set) is unaffected.

Fixes: 1ff82c13ea94 ("spi: tegra210-quad: Cache TRANS_STATUS in ISR for timeout handler")
Reported-by: Carol Soto <csoto@nvidia.com>
Link: https://github.com/NVIDIA/NV-Kernels/pull/579
Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index d8cca3aa276b..4a5933d8112c 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -1260,8 +1260,17 @@ static int tegra_qspi_handle_timeout(struct tegra_qspi *tqspi)
 	 * loaded the cache but before we masked).
 	 */
 	refreshed = smp_load_acquire(&tqspi->trans_status);
-	if (refreshed)
+	if (refreshed) {
 		status = refreshed;
+		/*
+		 * The drain observed a real ISR completion. Prefer the
+		 * ISR-published status_reg / tx_status / rx_status over
+		 * the pre-drain lost-IRQ snapshot, which may have been
+		 * taken after the ISR W1C-ed the hardware error bits and
+		 * therefore holds cleared values.
+		 */
+		lost_irq_snapshot = false;
+	}
 
 	if (!(status & QSPI_RDY)) {
 		retval = -ETIMEDOUT;
-- 
2.17.1

Re: [PATCH 1/2] spi: tegra210-quad: don't overwrite refreshed status with lost-IRQ snapshot

[Breno Leitao]

On Fri, Oct 16, 2026 at 01:25:19PM +0000, Vishwaroop A wrote:
> tegra_qspi_handle_timeout() captures a "lost IRQ" FIFO status snapshot
> when the ISR cache is empty on entry but QSPI_TRANS_STATUS shows the
> transfer completed. After masking the controller IRQ and draining the
> workqueue with cancel_work_sync(), it re-loads the ISR cache with
> smp_load_acquire(): if the drain observed a real ISR completion, the
> refreshed cache holds ISR-published status_reg, tx_status and rx_status
> that reflect the true hardware error state at IRQ time.
> 
> However, when both cache and snapshot are set, the code keeps the
> refreshed value in the local status variable but then unconditionally
> publishes the pre-drain lost_fifo_status / lost_tx_status /
> lost_rx_status snapshot into tqspi->status_reg / tx_status / rx_status
> via the lost_irq_snapshot branch. That snapshot is racy: if the ISR
> fires on another CPU between our live QSPI_TRANS_STATUS read and our
> live QSPI_FIFO_STATUS snapshot, the ISR W1Cs the hardware error bits
> before we sample them, so the snapshot may contain zeroed error
> fields. Overwriting the ISR-published values with these zeros makes
> the manual completion handler downstream silently accept a transfer
> that actually reported a FIFO error.
> 
> Clear lost_irq_snapshot when the refreshed cache is non-zero, so the
> ISR-published status wins over the pre-drain snapshot in that case.
> The lost-IRQ path (refreshed == 0 && live QSPI_RDY set) is unaffected.
> 
> Fixes: 1ff82c13ea94 ("spi: tegra210-quad: Cache TRANS_STATUS in ISR for timeout handler")
> Reported-by: Carol Soto <csoto@nvidia.com>
> Link: https://github.com/NVIDIA/NV-Kernels/pull/579
> Signed-off-by: Vishwaroop A <va@nvidia.com>

Reviewed-by: Breno Leitao <leitao@debian.org>

[PATCH 2/2] spi: tegra210-quad: Unmap DMA buffers on transfer timeout

[Vishwaroop A]

tegra_qspi_start_dma_based_transfer() maps the client TX/RX buffers with
dma_map_single() via tegra_qspi_dma_map_xfer(). The matching unmap only
happens in handle_dma_based_xfer(), which runs from the interrupt bottom
half.

When wait_for_completion_timeout() expires and tegra_qspi_handle_timeout()
cannot establish that the hardware finished, it returns an error without
the bottom half ever having run. Both transfer paths then abort the
transfer with tegra_qspi_dma_stop(), reset the controller and clear
curr_xfer, but never unmap the buffers. Every genuine transfer timeout
therefore leaks the streaming DMA mappings for that chunk, which
gradually exhausts the IOVA space on systems with an IOMMU in front of
QSPI.

Unmap the transfer after stopping DMA on both timeout error paths. The
unmap is placed after tegra_qspi_dma_stop() so the DMA engine is
quiesced before the mappings go away.

The recovery paths that do reach handle_dma_based_xfer() are unaffected:
that function always returns IRQ_HANDLED, so tegra_qspi_handle_timeout()
reports success and the caller's error path is not taken, leaving the
existing single unmap in place.

Fixes: 921fc1838fb0 ("spi: tegra210-quad: Add support for Tegra210 QSPI controller")
Cc: stable@vger.kernel.org
Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 4a5933d8112c..b77ad6990e7c 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -1488,10 +1488,12 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
 					dev_err(tqspi->dev, "transfer timeout\n");
 
 					/* Abort transfer by resetting pio/dma bit */
-					if (tqspi->is_curr_dma_xfer)
+					if (tqspi->is_curr_dma_xfer) {
 						tegra_qspi_dma_stop(tqspi);
-					else
+						tegra_qspi_dma_unmap_xfer(tqspi, xfer);
+					} else {
 						tegra_qspi_pio_stop(tqspi);
+					}
 
 					/* Reset controller if timeout happens */
 					tegra_qspi_reset(tqspi);
@@ -1605,8 +1607,10 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
 				WARN_ON(1);
 				dev_err(tqspi->dev, "transfer timeout\n");
 
-				if (tqspi->is_curr_dma_xfer)
+				if (tqspi->is_curr_dma_xfer) {
 					tegra_qspi_dma_stop(tqspi);
+					tegra_qspi_dma_unmap_xfer(tqspi, xfer);
+				}
 
 				tegra_qspi_handle_error(tqspi);
 				ret = -EIO;
-- 
2.17.1