From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nitin Garg Date: Fri, 27 Mar 2015 08:42:04 -0500 Subject: [U-Boot] bootcount: Add dcache flush to bootcount_store() In-Reply-To: <551558A2.3070508@denx.de> References: <1426063900-7267-2-git-send-email-sr@denx.de> <20150313134856.GA7447@bill-the-cat> <20150313143403.GE32541@bill-the-cat> <5507ED1D.7090005@keymile.com> <551558A2.3070508@denx.de> Message-ID: <55155E2C.1090304@freescale.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>>>> Reviewed-by: Tom Rini >>>> >>>> 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 2014-09-16 20:33:25 > Committer: Stefano Babic 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 > > 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