[PATCH] mmc: sdhci: Fix possible Synchronous Abort using PIO mode

[PATCH] mmc: sdhci: Fix possible Synchronous Abort using PIO mode

[Jonas Karlman]

When MMC_SDHCI_SDMA=y or MMC_SDHCI_ADMA=y and PIO mode is used
dma_unmap_single() is called on an unmapped address, 0x0. This may
result in a Synchronous Abort:

  ## Checking hash(es) for Image atf-1 ... sha256+ OK
  CMD_SEND:16
                  ARG                      0x00000200
                  MMC_RSP_R1,5,6,7         0x00000900
  CMD_SEND:18
                  ARG                      0x00004005
  "Synchronous Abort" handler, esr 0x96000147
  elr: 00000000400015bc lr : 0000000040012b4c
  x 0: 0000000000008000 x 1: 0000000000092600
  x 2: 0000000000000040 x 3: 000000000000003f
  x 4: 0000000000000030 x 5: 0000000000000001
  x 6: 0000000000000001 x 7: 0000000000000000
  x 8: 000000000000000a x 9: 0000000000000090
  x10: 0000000043dffc68 x11: 0000000043c00440
  x12: 0000000043c00440 x13: ffffffffbfe00000
  x14: 000000000000031c x15: 0000000240000000
  x16: 000000004001145c x17: 0000000000000032
  x18: 0000000043dffef0 x19: 0000000043c00000
  x20: 0000000043dffbc8 x21: 0000000000000000
  x22: 00000000000f3d95 x23: 0000000000000002
  x24: 0000000000000493 x25: 0000000000092600
  x26: 0000000000000001 x27: 0000000000000001
  x28: 0000000000000008 x29: 0000000043dffab0

  Code: d2800082 9ac32042 d1000443 8a230000 (d5087620)
  Resetting CPU ...

  resetting ...

Fix this by only dma_unmap_single() when DMA mode is used and
sdhci_prepare_dma() has been called to map host->start_addr.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/mmc/sdhci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 4833b5158c79..dc7f0724a7b6 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -177,8 +177,10 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
 	} while (!(stat & SDHCI_INT_DATA_END));
 
 #if (CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
-	dma_unmap_single(host->start_addr, data->blocks * data->blocksize,
-			 mmc_get_dma_dir(data));
+	if (host->flags & USE_DMA) {
+		dma_unmap_single(host->start_addr, data->blocks * data->blocksize,
+				 mmc_get_dma_dir(data));
+	}
 #endif
 
 	return 0;
-- 
2.48.1

Re: [PATCH] mmc: sdhci: Fix possible Synchronous Abort using PIO mode

[Peng Fan]

Hi Jonas,

On Thu, Jan 23, 2025 at 09:48:48PM +0000, Jonas Karlman wrote:
>When MMC_SDHCI_SDMA=y or MMC_SDHCI_ADMA=y and PIO mode is used
>dma_unmap_single() is called on an unmapped address, 0x0. This may
>result in a Synchronous Abort:
>
>  ## Checking hash(es) for Image atf-1 ... sha256+ OK
>  CMD_SEND:16
>                  ARG                      0x00000200
>                  MMC_RSP_R1,5,6,7         0x00000900
>  CMD_SEND:18
>                  ARG                      0x00004005
>  "Synchronous Abort" handler, esr 0x96000147
>  elr: 00000000400015bc lr : 0000000040012b4c
>  x 0: 0000000000008000 x 1: 0000000000092600
>  x 2: 0000000000000040 x 3: 000000000000003f
>  x 4: 0000000000000030 x 5: 0000000000000001
>  x 6: 0000000000000001 x 7: 0000000000000000
>  x 8: 000000000000000a x 9: 0000000000000090
>  x10: 0000000043dffc68 x11: 0000000043c00440
>  x12: 0000000043c00440 x13: ffffffffbfe00000
>  x14: 000000000000031c x15: 0000000240000000
>  x16: 000000004001145c x17: 0000000000000032
>  x18: 0000000043dffef0 x19: 0000000043c00000
>  x20: 0000000043dffbc8 x21: 0000000000000000
>  x22: 00000000000f3d95 x23: 0000000000000002
>  x24: 0000000000000493 x25: 0000000000092600
>  x26: 0000000000000001 x27: 0000000000000001
>  x28: 0000000000000008 x29: 0000000043dffab0
>
>  Code: d2800082 9ac32042 d1000443 8a230000 (d5087620)
>  Resetting CPU ...
>
>  resetting ...
>
>Fix this by only dma_unmap_single() when DMA mode is used and
>sdhci_prepare_dma() has been called to map host->start_addr.
>
>Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>---
> drivers/mmc/sdhci.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>index 4833b5158c79..dc7f0724a7b6 100644
>--- a/drivers/mmc/sdhci.c
>+++ b/drivers/mmc/sdhci.c
>@@ -177,8 +177,10 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
> 	} while (!(stat & SDHCI_INT_DATA_END));
> 
> #if (CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
>-	dma_unmap_single(host->start_addr, data->blocks * data->blocksize,
>-			 mmc_get_dma_dir(data));
>+	if (host->flags & USE_DMA) {
>+		dma_unmap_single(host->start_addr, data->blocks * data->blocksize,
>+				 mmc_get_dma_dir(data));
>+	}

Many platforms have MMC_SDHCI_SDMA enabled, but I not see USE_DMA is set.

So this change will make dma_unmap_single not being called for the platforms.

Will this introduce any issue?

Regards,
Peng

> #endif
> 
> 	return 0;
>-- 
>2.48.1
>

Re: [PATCH] mmc: sdhci: Fix possible Synchronous Abort using PIO mode

[Jonas Karlman]

Hi Peng,

On 2025-04-10 14:28, Peng Fan wrote:
> Hi Jonas,
> 
> On Thu, Jan 23, 2025 at 09:48:48PM +0000, Jonas Karlman wrote:
>> When MMC_SDHCI_SDMA=y or MMC_SDHCI_ADMA=y and PIO mode is used
>> dma_unmap_single() is called on an unmapped address, 0x0. This may
>> result in a Synchronous Abort:
>>
>>  ## Checking hash(es) for Image atf-1 ... sha256+ OK
>>  CMD_SEND:16
>>                  ARG                      0x00000200
>>                  MMC_RSP_R1,5,6,7         0x00000900
>>  CMD_SEND:18
>>                  ARG                      0x00004005
>>  "Synchronous Abort" handler, esr 0x96000147
>>  elr: 00000000400015bc lr : 0000000040012b4c
>>  x 0: 0000000000008000 x 1: 0000000000092600
>>  x 2: 0000000000000040 x 3: 000000000000003f
>>  x 4: 0000000000000030 x 5: 0000000000000001
>>  x 6: 0000000000000001 x 7: 0000000000000000
>>  x 8: 000000000000000a x 9: 0000000000000090
>>  x10: 0000000043dffc68 x11: 0000000043c00440
>>  x12: 0000000043c00440 x13: ffffffffbfe00000
>>  x14: 000000000000031c x15: 0000000240000000
>>  x16: 000000004001145c x17: 0000000000000032
>>  x18: 0000000043dffef0 x19: 0000000043c00000
>>  x20: 0000000043dffbc8 x21: 0000000000000000
>>  x22: 00000000000f3d95 x23: 0000000000000002
>>  x24: 0000000000000493 x25: 0000000000092600
>>  x26: 0000000000000001 x27: 0000000000000001
>>  x28: 0000000000000008 x29: 0000000043dffab0
>>
>>  Code: d2800082 9ac32042 d1000443 8a230000 (d5087620)
>>  Resetting CPU ...
>>
>>  resetting ...
>>
>> Fix this by only dma_unmap_single() when DMA mode is used and
>> sdhci_prepare_dma() has been called to map host->start_addr.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>> drivers/mmc/sdhci.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> index 4833b5158c79..dc7f0724a7b6 100644
>> --- a/drivers/mmc/sdhci.c
r>> +++ b/drivers/mmc/sdhci.c
>> @@ -177,8 +177,10 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
>> 	} while (!(stat & SDHCI_INT_DATA_END));
>>
>> #if (CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
>> -	dma_unmap_single(host->start_addr, data->blocks * data->blocksize,
>> -			 mmc_get_dma_dir(data));
>> +	if (host->flags & USE_DMA) {
>> +		dma_unmap_single(host->start_addr, data->blocks * data->blocksize,
>> +				 mmc_get_dma_dir(data));
>> +	}
> 
> Many platforms have MMC_SDHCI_SDMA enabled, but I not see USE_DMA is set.

  #define USE_DMA  (USE_SDMA | USE_ADMA | USE_ADMA64)

So any platform that enable support for xDMA should match USE_DMA.

For Rockchip there is a quirk enabled for SPL that may disable use of
xDMA even when xDMA is enabled at build time.

	/*
	 * Disable use of DMA and force use of PIO mode in SPL to fix an issue
	 * where loading part of TF-A into SRAM using DMA silently fails.
	 */
	if (IS_ENABLED(CONFIG_XPL_BUILD) &&
	    dev_read_bool(dev, "u-boot,spl-fifo-mode"))
		host->flags &= ~USE_DMA;

I.e. xDMA may be enabled at build time, and at runtime device may use
PIO mode.

> 
> So this change will make dma_unmap_single not being called for the platforms.

No, this will fix an uneven call to dma_map_single and dma_unmap_single.

dma_map_single() is currently called in sdhci_prepare_dma():
	if (host->flags & USE_DMA) { sdhci_prepare_dma() }

dma_unmap_single() is currently called in: 
	#if (CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))

This change just apply same (host->flags & USE_DMA) check that
indirectly wraps the dma_map_single() call.

> 
> Will this introduce any issue?

Not to my knowledge.

sdhci_send_command() initialize host->start_addr = 0, so the current
unconditional call to dma_unmap_single(host->start_addr, ...) in
sdhci_transfer_data() may cause the above Synchronous Abort due to
start_addr being 0.

Most boards may have DRAM 0x0 mapped, however the Rockchip RK3576 where
this issue was observed does not, its DRAM base start at 0x40000000.
Trying to unmap 0x0 ended up with a Synchronous Abort.

Regards,
Jonas

> 
> Regards,
> Peng
> 
>> #endif
>>
>> 	return 0;
>> -- 
>> 2.48.1
>>

Re: [PATCH] mmc: sdhci: Fix possible Synchronous Abort using PIO mode

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>


On Thu, 23 Jan 2025 21:48:48 +0000, Jonas Karlman wrote:
> When MMC_SDHCI_SDMA=y or MMC_SDHCI_ADMA=y and PIO mode is used
> dma_unmap_single() is called on an unmapped address, 0x0. This may
> result in a Synchronous Abort:
> 
>   ## Checking hash(es) for Image atf-1 ... sha256+ OK
>   CMD_SEND:16
>                   ARG                      0x00000200
>                   MMC_RSP_R1,5,6,7         0x00000900
>   CMD_SEND:18
>                   ARG                      0x00004005
>   "Synchronous Abort" handler, esr 0x96000147
>   elr: 00000000400015bc lr : 0000000040012b4c
>   x 0: 0000000000008000 x 1: 0000000000092600
>   x 2: 0000000000000040 x 3: 000000000000003f
>   x 4: 0000000000000030 x 5: 0000000000000001
>   x 6: 0000000000000001 x 7: 0000000000000000
>   x 8: 000000000000000a x 9: 0000000000000090
>   x10: 0000000043dffc68 x11: 0000000043c00440
>   x12: 0000000043c00440 x13: ffffffffbfe00000
>   x14: 000000000000031c x15: 0000000240000000
>   x16: 000000004001145c x17: 0000000000000032
>   x18: 0000000043dffef0 x19: 0000000043c00000
>   x20: 0000000043dffbc8 x21: 0000000000000000
>   x22: 00000000000f3d95 x23: 0000000000000002
>   x24: 0000000000000493 x25: 0000000000092600
>   x26: 0000000000000001 x27: 0000000000000001
>   x28: 0000000000000008 x29: 0000000043dffab0
> 
> [...]

Applied, thanks!

[1/1] mmc: sdhci: Fix possible Synchronous Abort using PIO mode
      commit: 2057bb4b5160a3668d4785adb6ce0f0d08325c48

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>