[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 at 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