[U-Boot] bootcount: Add dcache flush to bootcount_store()

[Nitin Garg]

Hi Stefan,
On 03/27/2015 08:18 AM, Stefan Roese wrote:
> Hi!
> 
> (added a few more people to the Cc list)
> 
> On 17.03.2015 10:00, Holger Brunck wrote:
>> Hi Tom,
>>
>> On 03/13/2015 03:34 PM, Tom Rini wrote:
>>> On Fri, Mar 13, 2015 at 09:48:56AM -0400, Tom Rini wrote:
>>>> On Wed, Mar 11, 2015 at 09:51:38AM +0100, Stefan Roese wrote:
>>>>
>>>>> Without this dcache_flush the updated bootcounter may not be saved to
>>>>> its location.
>>>>>
>>>>> This was detected on an iMX.6 platform using the OCRAM (internal SRAM)
>>>>> as bootcounter storage area. And issuing "reset" from within U-Boot
>>>>> cause the bootcounter to stay on its initial value.
>>>>>
>>>>> Signed-off-by: Stefan Roese <sr at denx.de>
>>>>> Reviewed-by: Tom Rini <trini at konsulko.com>
>>>>
>>>> OK, this breaks some platforms:
>>>>     powerpc:  +   TQM850L
>>>> +(TQM850L) drivers/built-in.o: In function `bootcount_store':
>>>> +(TQM850L) build/../drivers/bootcount/bootcount.c:64: undefined reference to `flush_dcache_range'
>>>> +(TQM850L) make[1]: *** [u-boot] Error 1
>>>> +(TQM850L) make: *** [sub-make] Error 2
>>>>
>>>> We'll see how many others have the same problem soon and then I'll
>>>> decide on nuking the old platforms of holding off on this change.
>>>
>>> Aside from the TQM 8xx family that Wolfgang owns we have mgcoge and
>>> mgcoge3ne also breaking from this
>>> (http://patchwork.ozlabs.org/patch/448849/) change.  Wolfgang, Holger,
>>> how do you want to proceed?  We either need cache operations or dropping
>>> bootcount from the platforms or dropping the platforms.
>>>
>>
>> we still would like to keep mgcoge and mgcoge3ne support. These boards are still
>> in maintenance. Unfortunately this week we are very busy. Next week Valentin or
>> myself have planned to find some time to look at this.
> 
> I just yesterday noticed this code for mx6:
> 
> ------------------------- arch/arm/cpu/armv7/mx6/soc.c -------------------------
> void enable_caches(void)
> {
> 	...
> 	/* Enable caching on OCRAM and ROM */
> 	mmu_set_region_dcache_behaviour(ROMCP_ARB_BASE_ADDR,
> 					ROMCP_ARB_END_ADDR,
> 					option);
> 	mmu_set_region_dcache_behaviour(IRAM_BASE_ADDR,
> 					IRAM_SIZE,
> 					option);
> 
> So we definitely have the dcache enabled on mx6 in the OCRAM. And this
> of course explains, why I need the cache flush operations in the
> bootcounter code.
> 
> I'm not really sure if we want this area to be cached though. This got
> introduced with this patch:
> 
> Author: Nitin Garg <nitin.garg at freescale.com>  2014-09-16 20:33:25
> Committer: Stefano Babic <sbabic at denx.de>  2014-09-22 16:21:04
> 
>     imx: Support i.MX6 High Assurance Boot authentication
>     
>     When CONFIG_SECURE_BOOT is enabled, the signed images
>     like kernel and dtb can be authenticated using iMX6 CAAM.
>     The added command hab_auth_img can be used for HAB
>     authentication of images. The command takes the image
>     DDR location, IVT (Image Vector Table) offset inside
>     image as parameters. Detailed info about signing images
>     can be found in Freescale AppNote AN4581.
>     
>     Signed-off-by: Nitin Garg <nitin.garg at freescale.com>
> 
> The cache stuff is not mentioned here in this commit log.
> Nitin, why did you enable the cache here? Performance reason
> only?
Seems like I missed mentioning this in the commit log, sorry.
We enabled the OCRAM d-cache since it helped HAB to
execute faster when authenticating OS image. The time taken 
to complete HAB improved by 3 times with d-cache enabled
for ROM and OCRAM. This is because the HAB uses various
regions of the OCRAM for data.

> 
> If the OCRAM was uncached (again), we could really drop my patch
> (this mail thread) with those flush calls. So how should we proceed?
> Make this OCRAM area uncached again?
> 
> Comments welcome...
> 
> Thanks,
> Stefan
> 
> BTW: I'm on vacation now until 8th April, so please don't expect
> any replies in this time.
> 
Regards,
Nitin