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

Lukasz Majewski
2018-05-08T07:41:08Z

On Tue, 08 May 2018 05:15:13 +0000
Alex Kiernan <alex.kiernan@gmail.com> wrote:

> On Wed, May 2, 2018 at 3:11 PM Lukasz Majewski <lukma@denx.de> wrote:
>
> > Those two functions can be used to provide easy bootcount
> > management.
>
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
>
> > Reviewed-by: Tom Rini <trini@konsulko.com>
> > Reviewed-by: Stefan Roese <sr@denx.de>
> > ---
>
> > Changes in v5:
> > - Provide parenthesis for #if defined(FOO) && ...
>
> > Changes in v4:
> > - Remove enum bootcount_context and replace it with checking
> > gd_t->flags (The GD_FLG_SPL_INIT is only set in SPL, it is cleared
> > in u-boot proper, so can be used as indication if we are in u-boot
> > or SPL).
> > - Do not call bootcount_store() twice when it is not needed.
> > - Call env_set_ulong("bootcount", bootcount); only in NON SPL
> > context - Boards with TINY_PRINTF (in newest mainline) will build
> > break as this
> function
> > requires simple_itoa() from vsprintf.c (now not always build in
> > SPL).
>
> > Changes in v3:
> > - Unify those functions to also work with common/autoboot.c code
> > - Add enum bootcount_context to distinguish between u-boot proper
> > and SPL
>
> > Changes in v2:
> > - None
>
> > include/bootcount.h | 47
> > +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47
> > insertions(+)
>
> > diff --git a/include/bootcount.h b/include/bootcount.h
> > index e3b3f7028e..a886c98f44 100644
> > --- a/include/bootcount.h
> > +++ b/include/bootcount.h
> > @@ -11,6 +11,8 @@
> > #include <asm/io.h>
> > #include <asm/byteorder.h>
>
> > +#if defined(CONFIG_SPL_BOOTCOUNT_LIMIT) ||
> defined(CONFIG_BOOTCOUNT_LIMIT)
> > +
> > #if !defined(CONFIG_SYS_BOOTCOUNT_LE) &&
> !defined(CONFIG_SYS_BOOTCOUNT_BE)
> > # if __BYTE_ORDER == __LITTLE_ENDIAN
> > # define CONFIG_SYS_BOOTCOUNT_LE
> > @@ -40,4 +42,49 @@ static inline u32 raw_bootcount_load(volatile
> > u32
> *addr)
> > return in_be32(addr);
> > }
> > #endif
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +static inline int bootcount_error(void)
> > +{
> > + unsigned long bootcount = bootcount_load();
> > + unsigned long bootlimit = env_get_ulong("bootlimit", 10, 0);
> > +
> > + if (bootlimit && bootcount > bootlimit) {
> > + printf("Warning: Bootlimit (%lu) exceeded.",
> > bootlimit);
> > + if (!(gd->flags & GD_FLG_SPL_INIT))
> > + printf(" Using altbootcmd.");
> > + printf("\n");
> > +
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +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 */
> }
>
> Also I suspect bootcount_store() will fail at link time on boards
> where the bootcount is stored in ext4?

I've run this patch set several times with travis-CI. No errors were
present (the travis-ci link is in the cover letter).

Maybe there are some out of tree boards, which use the bootcount in
some "odd" way....

>
> > +#if defined(CONFIG_SPL_BUILD)
> > && !defined(CONFIG_SPL_BOOTCOUNT_LIMIT) +void bootcount_store(ulong
> > a) {}; +ulong bootcount_load(void) { return 0; }
> > +#endif /* CONFIG_SPL_BUILD && !CONFIG_SPL_BOOTCOUNT_LIMIT */
> > +#else
> > +static inline int bootcount_error(void) { return 0; }
> > +static inline void bootcount_inc(void) {}
> > +#endif /* CONFIG_SPL_BOOTCOUNT_LIMIT || CONFIG_BOOTCOUNT_LIMIT */
> > #endif /* _BOOTCOUNT_H__ */
> > --
> > 2.11.0
>
>
>

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/1ac80274/attachment.sig>