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

[Lukasz Majewski <lukma@denx.de>]

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>