diff mbox

[U-Boot,1/2] MX28: SPI: Fix the DMA DCache race condition

Message ID 1346465280-985-1-git-send-email-marex@denx.de
State Accepted
Commit 88d155596879035532f8be05172d605965c733ed
Delegated to: Stefano Babic
Headers show

Commit Message

Marek Vasut Sept. 1, 2012, 2:07 a.m. UTC
This patch fixes dcache-related problem. The problem manifested
when dcache was enabled and the following command issued twice:

mw 0x42000000 0 0x4000 ; sf probe ; sf read 0x42000000 0x0 0x10000 ; sha1sum 0x42000000 0x10000

The SHA1 checksum was correct during the first call. Yet with
every subsequent call of the above command, it differed and was
wrong.

It turns out this was because of a race condition. On the first
time the command was called, no cacheline contained any data from
the destination memory location. The DMA transfered data into the
location and the cache above the location was invalidated. Then the
checksum was computed, but that meant the data were loaded into data
cache.

On any subsequent call, the DMA again transfered data into the same
destination. Yet during the transfer, some of the DCache lines were
evicted and written back into the main memory. Once the DMA transfer
completed, the data cache was invalidated over the memory location as
usual. But the data that were to be loaded back into the data cache
by subsequent SHA1 checksuming were corrupted.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Otavio Salvador <otavio@ossystems.com.br>
Cc: Stefano Babic <sbabic@denx.de>
---
 drivers/spi/mxs_spi.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

Comments

Stefano Babic Sept. 6, 2012, 12:20 p.m. UTC | #1
On 01/09/2012 04:07, Marek Vasut wrote:
> This patch fixes dcache-related problem. The problem manifested
> when dcache was enabled and the following command issued twice:
> 
> mw 0x42000000 0 0x4000 ; sf probe ; sf read 0x42000000 0x0 0x10000 ; sha1sum 0x42000000 0x10000
> 
> The SHA1 checksum was correct during the first call. Yet with
> every subsequent call of the above command, it differed and was
> wrong.
> 
> It turns out this was because of a race condition. On the first
> time the command was called, no cacheline contained any data from
> the destination memory location. The DMA transfered data into the
> location and the cache above the location was invalidated. Then the
> checksum was computed, but that meant the data were loaded into data
> cache.
> 
> On any subsequent call, the DMA again transfered data into the same
> destination. Yet during the transfer, some of the DCache lines were
> evicted and written back into the main memory. Once the DMA transfer
> completed, the data cache was invalidated over the memory location as
> usual. But the data that were to be loaded back into the data cache
> by subsequent SHA1 checksuming were corrupted.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Otavio Salvador <otavio@ossystems.com.br>
> Cc: Stefano Babic <sbabic@denx.de>
> ---

Applied to u-boot-imx, thanks.

Best regards,
Stefano Babic
diff mbox

Patch

diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
index 168dbe4..c399707 100644
--- a/drivers/spi/mxs_spi.c
+++ b/drivers/spi/mxs_spi.c
@@ -224,6 +224,7 @@  static int mxs_spi_xfer_dma(struct mxs_spi_slave *slave,
 	struct mxs_dma_desc *dp;
 	uint32_t ctrl0;
 	uint32_t cache_data_count;
+	const uint32_t dstart = (uint32_t)data;
 	int dmach;
 	int tl;
 
@@ -246,10 +247,12 @@  static int mxs_spi_xfer_dma(struct mxs_spi_slave *slave,
 	else
 		cache_data_count = length;
 
+	/* Flush data to DRAM so DMA can pick them up */
 	if (write)
-		/* Flush data to DRAM so DMA can pick them up */
-		flush_dcache_range((uint32_t)data,
-			(uint32_t)(data + cache_data_count));
+		flush_dcache_range(dstart, dstart + cache_data_count);
+
+	/* Invalidate the area, so no writeback into the RAM races with DMA */
+	invalidate_dcache_range(dstart, dstart + cache_data_count);
 
 	dmach = MXS_DMA_CHANNEL_AHB_APBH_SSP0 + slave->slave.bus;
 
@@ -310,10 +313,8 @@  static int mxs_spi_xfer_dma(struct mxs_spi_slave *slave,
 		return -EINVAL;
 
 	/* The data arrived into DRAM, invalidate cache over them */
-	if (!write) {
-		invalidate_dcache_range((uint32_t)data,
-			(uint32_t)(data + cache_data_count));
-	}
+	if (!write)
+		invalidate_dcache_range(dstart, dstart + cache_data_count);
 
 	return 0;
 }