[U-Boot] [PATCH v5 3/7] bootcount: Add function wrappers to handle bootcount increment and error checking

Lukasz Majewski
2018-05-08T07:38:45Z

Hi Alex, Stefan,

> On 08.05.2018 08:58, Alex Kiernan wrote:
>
> <snip>
>
> >>> +static inline void bootcount_inc(void)
> >>> +{
> >>> + unsigned long bootcount = bootcount_load();
> >>> +
> >>> + if (gd->flags & GD_FLG_SPL_INIT) {
> >>> + bootcount_store(++bootcount);
> >>> + return;
> >>> + }
> >>> +
> >>> +#ifndef CONFIG_SPL_BUILD
> >>> + /* Only increment bootcount when no bootcount support in
> >>> SPL */ +#ifndef CONFIG_SPL_BOOTCOUNT_LIMIT
> >>> + bootcount_store(++bootcount);
> >>> +#endif
> >>> + env_set_ulong("bootcount", bootcount);
> >>> +#endif /* !CONFIG_SPL_BUILD */
> >>> +}
> >>> +
> >
> >> I'm kinda confused by this code... isn't this equivalent.?
> >
> >> static inline void bootcount_inc(void)
> >> {
> >> unsigned long bootcount = bootcount_load();
> >
> >> bootcount_store(++bootcount);
> >> #ifndef CONFIG_SPL_BUILD
> >> env_set_ulong("bootcount", bootcount);
> >> #endif /* !CONFIG_SPL_BUILD */
> >> }
> >
> > I should've included my reasoning as I've got to be missing
> > something... if GD_FLG_SPL_INIT is always set when we get here in
> > SPL, then it's equivalent to the compile time guard. Which I think
> > says I don't understand the flow to how we get here, otherwise we
> > wouldn't need the runtime guard.
>
> When using with SPL and bootcounter support, this code will get
> called twice, first from the SPL, where the counter will get
> incremented. And second from main U-Boot, where we need to make
> sure, that the counter does not get incremented again, if SPL
> has already done so.

+1

>
> With your patch version, the bootcounter would get incremented
> twice in this case.
>
> Thanks,
> Stefan

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: <http://lists.denx.de/pipermail/u-boot/attachments/20180508/f0450882/attachment.sig>