From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukasz Majewski Date: Thu, 22 Feb 2018 14:03:28 +0100 Subject: [U-Boot] [PATCH] bootcount: flush after storing the bootcounter In-Reply-To: <258dfb89-e537-2ffb-ee3c-5b4bd72065e1@denx.de> References: <1519299041-19416-1-git-send-email-sbabic@denx.de> <20180222125224.3bd8bf14@jawa> <258dfb89-e537-2ffb-ee3c-5b4bd72065e1@denx.de> Message-ID: <20180222140328.2ec4179d@jawa> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Stefano, > Hi Lukasz, > > On 22/02/2018 12:52, Lukasz Majewski wrote: > > Hi Stefano, > > > >> If the bootcounter address is in a cached memory, > >> a flush of dcache must occur after updateing the bootcounter. > >> > >> Issue found on i.MX6 where bootcounter is put into the internal > >> (cached) IRAM. > >> > >> Signed-off-by: Stefano Babic > >> --- > >> drivers/bootcount/bootcount.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/drivers/bootcount/bootcount.c > >> b/drivers/bootcount/bootcount.c index d5ce450..48594a6 100644 > >> --- a/drivers/bootcount/bootcount.c > >> +++ b/drivers/bootcount/bootcount.c > >> @@ -59,6 +59,9 @@ __weak void bootcount_store(ulong a) > >> raw_bootcount_store(reg, a); > >> raw_bootcount_store(reg + 4, BOOTCOUNT_MAGIC); > >> #endif /* defined(CONFIG_SYS_BOOTCOUNT_SINGLEWORD */ > >> + flush_dcache_range(CONFIG_SYS_BOOTCOUNT_ADDR, > >> + CONFIG_SYS_BOOTCOUNT_ADDR + > >> + CONFIG_SYS_CACHELINE_SIZE); > > > > Is it safe to flush the whole cache line? > > Is there is some drawbacks ? I think not - as we are now in u-boot. (I do have patches, which add also support for bootcount in SPL - I will send them when Kconfig prerequisities go into mainline). > > flush_dcache_range() requires that addresses are aligned with > CONFIG_SYS_CACHELINE_SIZE. I cannot flush a single word. Yes. Correct. > > > > > For iMX6Q I do use SNVS_LPGR register (0x020CC068). > > It is a long story... :-) > > > It will preserve > > its content after reset caused by WDT (also reset command in > > u-boot). I also do use the SINGLEWORD to store "magic" and boot > > count in a single 32 bit number. > > > > I used it in the past - Heiko found issues with recent kernel versions > because kernel is using it. That means, after a rebootwe get what the > kernel has written in it, not the bootcounter anymore. You mean the SRC_GPRx registers? > > It looks like, too, that SNVS behavior changes between i.MX6 variants. > Even in manual, there are discordances (on i.MX6Q seems can be used, > on DL seems to be reserved,...). What I can say.... :/ > > Due to recent issues, I searched for a suitable place in IRAM. Ok. > > Anyway, this has nothing to do with the issue. If the storage with the > bootcounter is cached, it must be flushed. Yes. Of course. > > > > You may also want to consider using SRC_GPRx registers: > > https://community.nxp.com/message/985790?commentID=985790&et=watches.email.thread#comment-985790 > > See above, waiting for Heiko's comment,too. Ok. Maybe the reply from the NXP community is not complete... > > > > > As it shall be safe to use them for bootcount scenario. > > Rather, it looks like it is not safe. Or it was safe, it is not. Or it > depends on i.MX6 revisions.... It would be great if Heiko could share the problem with SRC_GPRx registers. I would like to know the root cause for the future usage. > > > However, I do > > prefer SNVS_LPGR. > > You're lucky you do not yet go into trouble :-) Devlopment by luck :-) > > > > >> } > >> > >> __weak ulong bootcount_load(void) > > > > > > > > Best regards, > Stefano > > Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 488 bytes Desc: OpenPGP digital signature URL: