[U-Boot] Problems with D-cache invalidation in Freescale eSDHC driver

[U-Boot] Problems with D-cache invalidation in Freescale eSDHC driver

[mario.six at gdsys.cc]

Hello,

I've been working on a QorIQ P1022 board (controlcenterd) to run the  
newest U-Boot on it, and I encountered some strange behavior.

During boot, we get these error messages

"
ERROR: Cannot import environment: errno = 12

at common/env_common.c:221/env_import()
*** Warning - import failed, using default environment

ERROR: Environment import failed: errno = 12

at common/env_common.c:123/set_default_env()
## Can't malloc 9 bytes
"

After bisecting, I found out that the commit 4683b22 (mmc:fsl_esdhc  
invalidate dcache before read) apparently causes this. With the  
additional d-cache invalidations in place, malloc and free calls fail.

Now, after some experimenting, I found out that deactivating the  
d-cache invalidation when reading the first sector is enough; that is,  
if one replaces the first invalidation

"
if (data->flags & MMC_DATA_READ)
	check_and_invalidate_dcache_range(cmd, data);
"

with

"
if (data->flags & MMC_DATA_READ && (cmd->cmdidx !=  
MMC_CMD_READ_SINGLE_BLOCK || cmd->cmdarg != 0))
	check_and_invalidate_dcache_range(cmd, data);
"

in fsl_esdhc.c, it seems to work, but I have no idea why that is the case.

Any ideas would be greatly appreciated.

Best regards,
Mario

[U-Boot] Problems with D-cache invalidation in Freescale eSDHC driver

[Peng Fan]

Hi Mario,

On Fri, Mar 18, 2016 at 09:16:48AM +0100, mario.six at gdsys.cc wrote:
>
>Hello,
>
>I've been working on a QorIQ P1022 board (controlcenterd) to run the newest
>U-Boot on it, and I encountered some strange behavior.
>
>During boot, we get these error messages
>
>"
>ERROR: Cannot import environment: errno = 12
>
>at common/env_common.c:221/env_import()
>*** Warning - import failed, using default environment
>
>ERROR: Environment import failed: errno = 12
>
>at common/env_common.c:123/set_default_env()
>## Can't malloc 9 bytes
>"
>
>After bisecting, I found out that the commit 4683b22 (mmc:fsl_esdhc
>invalidate dcache before read) apparently causes this. With the additional
>d-cache invalidations in place, malloc and free calls fail.
>
>Now, after some experimenting, I found out that deactivating the d-cache
>invalidation when reading the first sector is enough; that is, if one
>replaces the first invalidation
>
>"
>if (data->flags & MMC_DATA_READ)
>	check_and_invalidate_dcache_range(cmd, data);
>"
>
>with
>
>"
>if (data->flags & MMC_DATA_READ && (cmd->cmdidx != MMC_CMD_READ_SINGLE_BLOCK
>|| cmd->cmdarg != 0))
>	check_and_invalidate_dcache_range(cmd, data);
>"
>
>in fsl_esdhc.c, it seems to work, but I have no idea why that is the case.
>
>Any ideas would be greatly appreciated.

Before you chaning:
"
if (data->flags & MMC_DATA_READ)
	check_and_invalidate_dcache_range(cmd, data);
"

can you please try this, https://patchwork.ozlabs.org/patch/511889/ and
kindly give some feedback whether the patch can fix you issue or not?

I strongly agree that U-Boot should flow the Linux DMA flow to avoid
potential write back and speculative read which may cause DMA data
be polluted.

Anyway please first try first invalidate L2, then invalidate L1.

I plan to pick the upper patch or rework for DMA usage, but have not
began the work (:

Thanks,
Peng.

>
>Best regards,
>Mario
>
>
>_______________________________________________
>U-Boot mailing list
>U-Boot at lists.denx.de
>http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] Problems with D-cache invalidation in Freescale eSDHC driver

[mario.six at gdsys.cc]

Hi Peng,

Quoting Peng Fan <van.freenix@gmail.com>:

> Hi Mario,
>
> On Fri, Mar 18, 2016 at 09:16:48AM +0100, mario.six at gdsys.cc wrote:
>>
>> Hello,
>>
>> I've been working on a QorIQ P1022 board (controlcenterd) to run the newest
>> U-Boot on it, and I encountered some strange behavior.
>>
>> During boot, we get these error messages
>>
>> "
>> ERROR: Cannot import environment: errno = 12
>>
>> at common/env_common.c:221/env_import()
>> *** Warning - import failed, using default environment
>>
>> ERROR: Environment import failed: errno = 12
>>
>> at common/env_common.c:123/set_default_env()
>> ## Can't malloc 9 bytes
>> "
>>
>> After bisecting, I found out that the commit 4683b22 (mmc:fsl_esdhc
>> invalidate dcache before read) apparently causes this. With the additional
>> d-cache invalidations in place, malloc and free calls fail.
>>
>> Now, after some experimenting, I found out that deactivating the d-cache
>> invalidation when reading the first sector is enough; that is, if one
>> replaces the first invalidation
>>
>> "
>> if (data->flags & MMC_DATA_READ)
>> 	check_and_invalidate_dcache_range(cmd, data);
>> "
>>
>> with
>>
>> "
>> if (data->flags & MMC_DATA_READ && (cmd->cmdidx != MMC_CMD_READ_SINGLE_BLOCK
>> || cmd->cmdarg != 0))
>> 	check_and_invalidate_dcache_range(cmd, data);
>> "
>>
>> in fsl_esdhc.c, it seems to work, but I have no idea why that is the case.
>>
>> Any ideas would be greatly appreciated.
>
> Before you chaning:
> "
> if (data->flags & MMC_DATA_READ)
> 	check_and_invalidate_dcache_range(cmd, data);
> "
>
> can you please try this, https://patchwork.ozlabs.org/patch/511889/ and
> kindly give some feedback whether the patch can fix you issue or not?
>
> I strongly agree that U-Boot should flow the Linux DMA flow to avoid
> potential write back and speculative read which may cause DMA data
> be polluted.
>
> Anyway please first try first invalidate L2, then invalidate L1.
>
> I plan to pick the upper patch or rework for DMA usage, but have not
> began the work (:

Thanks for the reply!

I tried to add the L2 invalidation from release.S into the
invalidate_dcache_range function from ppccache.S:

"

Subject: [PATCH] Invalidate L2 cache before L1 cache

---
  arch/powerpc/lib/ppccache.S | 9 +++++++++
  1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/lib/ppccache.S b/arch/powerpc/lib/ppccache.S
index b96dbc6..3f68be2 100644
--- a/arch/powerpc/lib/ppccache.S
+++ b/arch/powerpc/lib/ppccache.S
@@ -87,6 +87,15 @@ _GLOBAL(flush_dcache_range)
   * invalidate_dcache_range(unsigned long start, unsigned long stop)
   */
  _GLOBAL(invalidate_dcache_range)
+	msync
+	lis	r2,(L2CSR0_L2FI|L2CSR0_L2LFC)@h
+	ori	r2,r2,(L2CSR0_L2FI|L2CSR0_L2LFC)@l
+	mtspr	SPRN_L2CSR0,r2
+2:
+	mfspr	r5,SPRN_L2CSR0
+	and.	r1,r5,r2
+	bne	2b
+
  	li	r5,L1_CACHE_BYTES-1
  	andc	r3,r3,r5
  	subf	r4,r3,r4
--
1.8.3

"

That had the effect of crashing the board on the first MMC access, sadly.

But, this had me thinking. Correct me if I'm wrong, since I'm in *no  
way* a DMA
expert, but the logic currently works as follows:

1. Before the DMA transfer, invalidate the d-cache, so no stale data in the
cache can pollute the result of the DMA transfer.
2. Do the DMA transfer.
3. Invalidate the d-cache again, so the CPU doesn't read stale data from the
cache.

Shouldn't the cache be *flushed* in the first step? The DMA transfer didn't
happen yet, so the data in the cache is "legitimate," in the sense that it
originated from regular usage of the cache systen, and writing the data from
the cache back to the memory doesn't overwrite something we don't want it to.

I did just that, and replaced the invalidation with a flush, and it seems to
fix my problem, at least :) (I included the patch at the end of the message).

Thanks again for the help!

Best regards,
Mario

-- >8 --

Subject: [PATCH] Replace cache invalidation with flush before DMA transfer

---
  drivers/mmc/fsl_esdhc.c | 25 ++++++++++++++++++++++++-
  1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index ea5f4bf..aa5b29e 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -302,6 +302,29 @@ static void check_and_invalidate_dcache_range
  	invalidate_dcache_range(start, end);
  }

+static void check_and_flush_dcache_range
+	(struct mmc_cmd *cmd,
+	 struct mmc_data *data) {
+#ifdef CONFIG_FSL_LAYERSCAPE
+	unsigned start = 0;
+#else
+	unsigned start = (unsigned)data->dest ;
+#endif
+	unsigned size = roundup(ARCH_DMA_MINALIGN,
+				data->blocks*data->blocksize);
+	unsigned end = start+size ;
+#ifdef CONFIG_FSL_LAYERSCAPE
+	dma_addr_t addr;
+
+	addr = virt_to_phys((void *)(data->dest));
+	if (upper_32_bits(addr))
+		printf("Error found for upper 32 bits\n");
+	else
+		start = lower_32_bits(addr);
+#endif
+	flush_dcache_range(start, end);
+}
+
  /*
   * Sends a command out on the bus.  Takes the mmc pointer,
   * a command pointer, and an optional data pointer.
@@ -346,7 +369,7 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd  
*cmd, struct mmc_data *data)
  			return err;

  		if (data->flags & MMC_DATA_READ)
-			check_and_invalidate_dcache_range(cmd, data);
+			check_and_flush_dcache_range(cmd, data);
  	}

  	/* Figure out the transfer arguments */
--
1.8.3

>
> Thanks,
> Peng.
>
>>
>> Best regards,
>> Mario

[U-Boot] Problems with D-cache invalidation in Freescale eSDHC driver

[Peng Fan]

Hi Maro,

+More people. There maybe more ideas about this.

On Mon, Mar 21, 2016 at 10:32:46AM +0100, mario.six at gdsys.cc wrote:
>Hi Peng,
>
>Quoting Peng Fan <van.freenix@gmail.com>:
>
>>Hi Mario,
>>
>>On Fri, Mar 18, 2016 at 09:16:48AM +0100, mario.six at gdsys.cc wrote:
>>>
>>>Hello,
>>>
>>>I've been working on a QorIQ P1022 board (controlcenterd) to run the newest
>>>U-Boot on it, and I encountered some strange behavior.
>>>
>>>During boot, we get these error messages
>>>
>>>"
>>>ERROR: Cannot import environment: errno = 12
>>>
>>>at common/env_common.c:221/env_import()
>>>*** Warning - import failed, using default environment
>>>
>>>ERROR: Environment import failed: errno = 12
>>>
>>>at common/env_common.c:123/set_default_env()
>>>## Can't malloc 9 bytes
>>>"
>>>
>>>After bisecting, I found out that the commit 4683b22 (mmc:fsl_esdhc
>>>invalidate dcache before read) apparently causes this. With the additional
>>>d-cache invalidations in place, malloc and free calls fail.
>>>
>>>Now, after some experimenting, I found out that deactivating the d-cache
>>>invalidation when reading the first sector is enough; that is, if one
>>>replaces the first invalidation
>>>
>>>"
>>>if (data->flags & MMC_DATA_READ)
>>>	check_and_invalidate_dcache_range(cmd, data);
>>>"
>>>
>>>with
>>>
>>>"
>>>if (data->flags & MMC_DATA_READ && (cmd->cmdidx != MMC_CMD_READ_SINGLE_BLOCK
>>>|| cmd->cmdarg != 0))
>>>	check_and_invalidate_dcache_range(cmd, data);
>>>"
>>>
>>>in fsl_esdhc.c, it seems to work, but I have no idea why that is the case.
>>>
>>>Any ideas would be greatly appreciated.
>>
>>Before you chaning:
>>"
>>if (data->flags & MMC_DATA_READ)
>>	check_and_invalidate_dcache_range(cmd, data);
>>"
>>
>>can you please try this, https://patchwork.ozlabs.org/patch/511889/ and
>>kindly give some feedback whether the patch can fix you issue or not?
>>
>>I strongly agree that U-Boot should flow the Linux DMA flow to avoid
>>potential write back and speculative read which may cause DMA data
>>be polluted.
>>
>>Anyway please first try first invalidate L2, then invalidate L1.
>>
>>I plan to pick the upper patch or rework for DMA usage, but have not
>>began the work (:
>
>Thanks for the reply!
>
>I tried to add the L2 invalidation from release.S into the
>invalidate_dcache_range function from ppccache.S:

Oh. I missed you are using ppc.
Sorry. Actually the flow I suggested is for ARM.

>
>"
>
>Subject: [PATCH] Invalidate L2 cache before L1 cache
>
>---
> arch/powerpc/lib/ppccache.S | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
>diff --git a/arch/powerpc/lib/ppccache.S b/arch/powerpc/lib/ppccache.S
>index b96dbc6..3f68be2 100644
>--- a/arch/powerpc/lib/ppccache.S
>+++ b/arch/powerpc/lib/ppccache.S
>@@ -87,6 +87,15 @@ _GLOBAL(flush_dcache_range)
>  * invalidate_dcache_range(unsigned long start, unsigned long stop)
>  */
> _GLOBAL(invalidate_dcache_range)
>+	msync
>+	lis	r2,(L2CSR0_L2FI|L2CSR0_L2LFC)@h
>+	ori	r2,r2,(L2CSR0_L2FI|L2CSR0_L2LFC)@l
>+	mtspr	SPRN_L2CSR0,r2
>+2:
>+	mfspr	r5,SPRN_L2CSR0
>+	and.	r1,r5,r2
>+	bne	2b
>+
> 	li	r5,L1_CACHE_BYTES-1
> 	andc	r3,r3,r5
> 	subf	r4,r3,r4
>--
>1.8.3
>
>"
>
>That had the effect of crashing the board on the first MMC access, sadly.
>
>But, this had me thinking. Correct me if I'm wrong, since I'm in *no way* a
>DMA
>expert, but the logic currently works as follows:
>
>1. Before the DMA transfer, invalidate the d-cache, so no stale data in the
>cache can pollute the result of the DMA transfer.
>2. Do the DMA transfer.
>3. Invalidate the d-cache again, so the CPU doesn't read stale data from the
>cache.
>
>Shouldn't the cache be *flushed* in the first step? The DMA transfer didn't

To ARM, invalidate is enough, no need to flush and the flow is
1. Invalidate L1, then invalidate L2 to fix potential write back.
2. DMA read
3. Invalidate L2, then invalidate L1 to fix potential speculative read.

But in uboot, we do not have dma api, only use invalidate_cache_range to do
the invalidation and the flow does not follow ARM recommendations after DMA read.

I have no knowledge of PPC (:

I think you need to check the flow of dma_map_area and dmp_unmap_area, then
you may get the idea of how ppc handles DMA, then use it in this fsl_esdhc
driver. Hope this is useful.

Regards,
Peng.

>happen yet, so the data in the cache is "legitimate," in the sense that it
>originated from regular usage of the cache systen, and writing the data from
>the cache back to the memory doesn't overwrite something we don't want it to.
>
>I did just that, and replaced the invalidation with a flush, and it seems to
>fix my problem, at least :) (I included the patch at the end of the message).
>
>Thanks again for the help!
>
>Best regards,
>Mario
>
>-- >8 --
>
>Subject: [PATCH] Replace cache invalidation with flush before DMA transfer
>
>---
> drivers/mmc/fsl_esdhc.c | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
>index ea5f4bf..aa5b29e 100644
>--- a/drivers/mmc/fsl_esdhc.c
>+++ b/drivers/mmc/fsl_esdhc.c
>@@ -302,6 +302,29 @@ static void check_and_invalidate_dcache_range
> 	invalidate_dcache_range(start, end);
> }
>
>+static void check_and_flush_dcache_range
>+	(struct mmc_cmd *cmd,
>+	 struct mmc_data *data) {
>+#ifdef CONFIG_FSL_LAYERSCAPE
>+	unsigned start = 0;
>+#else
>+	unsigned start = (unsigned)data->dest ;
>+#endif
>+	unsigned size = roundup(ARCH_DMA_MINALIGN,
>+				data->blocks*data->blocksize);
>+	unsigned end = start+size ;
>+#ifdef CONFIG_FSL_LAYERSCAPE
>+	dma_addr_t addr;
>+
>+	addr = virt_to_phys((void *)(data->dest));
>+	if (upper_32_bits(addr))
>+		printf("Error found for upper 32 bits\n");
>+	else
>+		start = lower_32_bits(addr);
>+#endif
>+	flush_dcache_range(start, end);
>+}
>+
> /*
>  * Sends a command out on the bus.  Takes the mmc pointer,
>  * a command pointer, and an optional data pointer.
>@@ -346,7 +369,7 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>struct mmc_data *data)
> 			return err;
>
> 		if (data->flags & MMC_DATA_READ)
>-			check_and_invalidate_dcache_range(cmd, data);
>+			check_and_flush_dcache_range(cmd, data);
> 	}
>
> 	/* Figure out the transfer arguments */
>--
>1.8.3
>
>>
>>Thanks,
>>Peng.
>>
>>>
>>>Best regards,
>>>Mario
>

[U-Boot] Problems with D-cache invalidation in Freescale eSDHC driver

[york sun]

On 03/21/2016 03:11 AM, Peng Fan wrote:
> Hi Maro,
> 
> +More people. There maybe more ideas about this.
> 
> On Mon, Mar 21, 2016 at 10:32:46AM +0100, mario.six at gdsys.cc wrote:
>> Hi Peng,
>>
>> Quoting Peng Fan <van.freenix@gmail.com>:
>>
>>> Hi Mario,
>>>
>>> On Fri, Mar 18, 2016 at 09:16:48AM +0100, mario.six at gdsys.cc wrote:
>>>>
>>>> Hello,
>>>>
>>>> I've been working on a QorIQ P1022 board (controlcenterd) to run the newest
>>>> U-Boot on it, and I encountered some strange behavior.
>>>>
>>>> During boot, we get these error messages
>>>>
>>>> "
>>>> ERROR: Cannot import environment: errno = 12
>>>>
>>>> at common/env_common.c:221/env_import()
>>>> *** Warning - import failed, using default environment
>>>>
>>>> ERROR: Environment import failed: errno = 12
>>>>
>>>> at common/env_common.c:123/set_default_env()
>>>> ## Can't malloc 9 bytes
>>>> "
>>>>
>>>> After bisecting, I found out that the commit 4683b22 (mmc:fsl_esdhc
>>>> invalidate dcache before read) apparently causes this. With the additional
>>>> d-cache invalidations in place, malloc and free calls fail.
>>>>
>>>> Now, after some experimenting, I found out that deactivating the d-cache
>>>> invalidation when reading the first sector is enough; that is, if one
>>>> replaces the first invalidation
>>>>
>>>> "
>>>> if (data->flags & MMC_DATA_READ)
>>>> 	check_and_invalidate_dcache_range(cmd, data);
>>>> "
>>>>
>>>> with
>>>>
>>>> "
>>>> if (data->flags & MMC_DATA_READ && (cmd->cmdidx != MMC_CMD_READ_SINGLE_BLOCK
>>>> || cmd->cmdarg != 0))
>>>> 	check_and_invalidate_dcache_range(cmd, data);
>>>> "
>>>>
>>>> in fsl_esdhc.c, it seems to work, but I have no idea why that is the case.
>>>>
>>>> Any ideas would be greatly appreciated.
>>>
>>> Before you chaning:
>>> "
>>> if (data->flags & MMC_DATA_READ)
>>> 	check_and_invalidate_dcache_range(cmd, data);
>>> "
>>>
>>> can you please try this, https://patchwork.ozlabs.org/patch/511889/ and
>>> kindly give some feedback whether the patch can fix you issue or not?
>>>
>>> I strongly agree that U-Boot should flow the Linux DMA flow to avoid
>>> potential write back and speculative read which may cause DMA data
>>> be polluted.
>>>
>>> Anyway please first try first invalidate L2, then invalidate L1.
>>>
>>> I plan to pick the upper patch or rework for DMA usage, but have not
>>> began the work (:
>>
>> Thanks for the reply!
>>
>> I tried to add the L2 invalidation from release.S into the
>> invalidate_dcache_range function from ppccache.S:
> 
> Oh. I missed you are using ppc.
> Sorry. Actually the flow I suggested is for ARM.
> 
>>
>> "
>>
>> Subject: [PATCH] Invalidate L2 cache before L1 cache
>>
>> ---
>> arch/powerpc/lib/ppccache.S | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/powerpc/lib/ppccache.S b/arch/powerpc/lib/ppccache.S
>> index b96dbc6..3f68be2 100644
>> --- a/arch/powerpc/lib/ppccache.S
>> +++ b/arch/powerpc/lib/ppccache.S
>> @@ -87,6 +87,15 @@ _GLOBAL(flush_dcache_range)
>>  * invalidate_dcache_range(unsigned long start, unsigned long stop)
>>  */
>> _GLOBAL(invalidate_dcache_range)
>> +	msync
>> +	lis	r2,(L2CSR0_L2FI|L2CSR0_L2LFC)@h
>> +	ori	r2,r2,(L2CSR0_L2FI|L2CSR0_L2LFC)@l
>> +	mtspr	SPRN_L2CSR0,r2
>> +2:
>> +	mfspr	r5,SPRN_L2CSR0
>> +	and.	r1,r5,r2
>> +	bne	2b
>> +
>> 	li	r5,L1_CACHE_BYTES-1
>> 	andc	r3,r3,r5
>> 	subf	r4,r3,r4
>> --
>> 1.8.3
>>
>> "
>>
>> That had the effect of crashing the board on the first MMC access, sadly.
>>
>> But, this had me thinking. Correct me if I'm wrong, since I'm in *no way* a
>> DMA
>> expert, but the logic currently works as follows:
>>
>> 1. Before the DMA transfer, invalidate the d-cache, so no stale data in the
>> cache can pollute the result of the DMA transfer.
>> 2. Do the DMA transfer.
>> 3. Invalidate the d-cache again, so the CPU doesn't read stale data from the
>> cache.
>>
>> Shouldn't the cache be *flushed* in the first step? The DMA transfer didn't
> 
> To ARM, invalidate is enough, no need to flush and the flow is
> 1. Invalidate L1, then invalidate L2 to fix potential write back.
> 2. DMA read
> 3. Invalidate L2, then invalidate L1 to fix potential speculative read.
> 
> But in uboot, we do not have dma api, only use invalidate_cache_range to do
> the invalidation and the flow does not follow ARM recommendations after DMA read.
> 
> I have no knowledge of PPC (:

Guys,

This may be related to a similar issue I saw earlier
http://lists.denx.de/pipermail/u-boot/2015-December/236272.html

A invalidate_cache_range function was added by
commitac337168ad81a18e768e5e3cfff8d229adeb2b25 (patch
http://patchwork.ozlabs.org/patch/455439). It looks good, but caused problem
when used on e500v2. This function was no-op for 512x, 83xx and 85xx before. I
was thinking to revert partially to keep the function only for 4xx and 86xx.

York

[U-Boot] Problems with D-cache invalidation in Freescale eSDHC driver

[Mario Six]

Hi York,

Quoting york sun <york.sun@nxp.com>:

> On 03/21/2016 03:11 AM, Peng Fan wrote:
>> Hi Maro,
>>
>> +More people. There maybe more ideas about this.
>>
>> On Mon, Mar 21, 2016 at 10:32:46AM +0100, mario.six at gdsys.cc wrote:
>>> Hi Peng,
>>>
>>> Quoting Peng Fan <van.freenix@gmail.com>:
>>>
>>>> Hi Mario,
>>>>
>>>> On Fri, Mar 18, 2016 at 09:16:48AM +0100, mario.six at gdsys.cc wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> I've been working on a QorIQ P1022 board (controlcenterd) to run  
>>>>> the newest
>>>>> U-Boot on it, and I encountered some strange behavior.
>>>>>
>>>>> During boot, we get these error messages
>>>>>
>>>>> "
>>>>> ERROR: Cannot import environment: errno = 12
>>>>>
>>>>> at common/env_common.c:221/env_import()
>>>>> *** Warning - import failed, using default environment
>>>>>
>>>>> ERROR: Environment import failed: errno = 12
>>>>>
>>>>> at common/env_common.c:123/set_default_env()
>>>>> ## Can't malloc 9 bytes
>>>>> "
>>>>>
>>>>> After bisecting, I found out that the commit 4683b22 (mmc:fsl_esdhc
>>>>> invalidate dcache before read) apparently causes this. With the  
>>>>> additional
>>>>> d-cache invalidations in place, malloc and free calls fail.
>>>>>
>>>>> Now, after some experimenting, I found out that deactivating the d-cache
>>>>> invalidation when reading the first sector is enough; that is, if one
>>>>> replaces the first invalidation
>>>>>
>>>>> "
>>>>> if (data->flags & MMC_DATA_READ)
>>>>> 	check_and_invalidate_dcache_range(cmd, data);
>>>>> "
>>>>>
>>>>> with
>>>>>
>>>>> "
>>>>> if (data->flags & MMC_DATA_READ && (cmd->cmdidx !=  
>>>>> MMC_CMD_READ_SINGLE_BLOCK
>>>>> || cmd->cmdarg != 0))
>>>>> 	check_and_invalidate_dcache_range(cmd, data);
>>>>> "
>>>>>
>>>>> in fsl_esdhc.c, it seems to work, but I have no idea why that is  
>>>>> the case.
>>>>>
>>>>> Any ideas would be greatly appreciated.
>>>>
>>>> Before you chaning:
>>>> "
>>>> if (data->flags & MMC_DATA_READ)
>>>> 	check_and_invalidate_dcache_range(cmd, data);
>>>> "
>>>>
>>>> can you please try this, https://patchwork.ozlabs.org/patch/511889/ and
>>>> kindly give some feedback whether the patch can fix you issue or not?
>>>>
>>>> I strongly agree that U-Boot should flow the Linux DMA flow to avoid
>>>> potential write back and speculative read which may cause DMA data
>>>> be polluted.
>>>>
>>>> Anyway please first try first invalidate L2, then invalidate L1.
>>>>
>>>> I plan to pick the upper patch or rework for DMA usage, but have not
>>>> began the work (:
>>>
>>> Thanks for the reply!
>>>
>>> I tried to add the L2 invalidation from release.S into the
>>> invalidate_dcache_range function from ppccache.S:
>>
>> Oh. I missed you are using ppc.
>> Sorry. Actually the flow I suggested is for ARM.
>>
>>>
>>> "
>>>
>>> Subject: [PATCH] Invalidate L2 cache before L1 cache
>>>
>>> ---
>>> arch/powerpc/lib/ppccache.S | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/arch/powerpc/lib/ppccache.S b/arch/powerpc/lib/ppccache.S
>>> index b96dbc6..3f68be2 100644
>>> --- a/arch/powerpc/lib/ppccache.S
>>> +++ b/arch/powerpc/lib/ppccache.S
>>> @@ -87,6 +87,15 @@ _GLOBAL(flush_dcache_range)
>>>  * invalidate_dcache_range(unsigned long start, unsigned long stop)
>>>  */
>>> _GLOBAL(invalidate_dcache_range)
>>> +	msync
>>> +	lis	r2,(L2CSR0_L2FI|L2CSR0_L2LFC)@h
>>> +	ori	r2,r2,(L2CSR0_L2FI|L2CSR0_L2LFC)@l
>>> +	mtspr	SPRN_L2CSR0,r2
>>> +2:
>>> +	mfspr	r5,SPRN_L2CSR0
>>> +	and.	r1,r5,r2
>>> +	bne	2b
>>> +
>>> 	li	r5,L1_CACHE_BYTES-1
>>> 	andc	r3,r3,r5
>>> 	subf	r4,r3,r4
>>> --
>>> 1.8.3
>>>
>>> "
>>>
>>> That had the effect of crashing the board on the first MMC access, sadly.
>>>
>>> But, this had me thinking. Correct me if I'm wrong, since I'm in *no way* a
>>> DMA
>>> expert, but the logic currently works as follows:
>>>
>>> 1. Before the DMA transfer, invalidate the d-cache, so no stale data in the
>>> cache can pollute the result of the DMA transfer.
>>> 2. Do the DMA transfer.
>>> 3. Invalidate the d-cache again, so the CPU doesn't read stale  
>>> data from the
>>> cache.
>>>
>>> Shouldn't the cache be *flushed* in the first step? The DMA transfer didn't
>>
>> To ARM, invalidate is enough, no need to flush and the flow is
>> 1. Invalidate L1, then invalidate L2 to fix potential write back.
>> 2. DMA read
>> 3. Invalidate L2, then invalidate L1 to fix potential speculative read.
>>
>> But in uboot, we do not have dma api, only use invalidate_cache_range to do
>> the invalidation and the flow does not follow ARM recommendations  
>> after DMA read.
>>
>> I have no knowledge of PPC (:
>
> Guys,
>
> This may be related to a similar issue I saw earlier
> http://lists.denx.de/pipermail/u-boot/2015-December/236272.html
>
> A invalidate_cache_range function was added by
> commitac337168ad81a18e768e5e3cfff8d229adeb2b25 (patch
> http://patchwork.ozlabs.org/patch/455439). It looks good, but caused problem
> when used on e500v2. This function was no-op for 512x, 83xx and 85xx  
> before. I
> was thinking to revert partially to keep the function only for 4xx and 86xx.
>
> York

Yes, might be for the best to partially revert the commit. Though, I do agree
with Scott's opinion from the thread you mentioned that it would be good to
find out what the actual problem is.

Best regards,

Mario