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

[Alex Kiernan]

On Tue, May 8, 2018 at 8:11 AM Stefan Roese <sr at denx.de> wrote:

> 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.

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


Ahh... thank you. That was the important piece!

-- 
Alex Kiernan